← Back to team overview

maria-developers team mailing list archive

Re: [Commits] 43e2a78f2a0: Remove MYSQL_COMPRESSION.

 

Hi,

On Mon, Jan 16, 2017 at 2:10 PM, <marko.makela@xxxxxxxxxxx> wrote:

>
> diff --git a/storage/innobase/btr/btr0btr.cc b/storage/innobase/btr/
> btr0btr.cc
> index d1d9dfe64fe..1fb5cb06949 100644
> --- a/storage/innobase/btr/btr0btr.cc
> +++ b/storage/innobase/btr/btr0btr.cc
> @@ -215,10 +215,6 @@ btr_root_get(
>         buf_block_t* root = btr_root_block_get(index, RW_SX_LATCH,
>                                                mtr);
>
> -       if (root && root->page.encrypted == true) {
> -               root = NULL;
> -       }
> -
>         return(root ? buf_block_get_frame(root) : NULL);
>

This is unrelated to MySQL encryption, it is related to MariaDB encryption,
not sure if btr_root_block_get
would always return NULL when page is encrypted, upper code is peppered
with assertions, thus this might
not be safe.



>
> -       if (ptype == FIL_PAGE_PAGE_COMPRESSED_ENCRYPTED) {
> -               header_len += FIL_PAGE_COMPRESSION_METHOD_SIZE;
> -       }
> -
> -       /* Do not try to uncompressed pages that are not compressed */
> -       if (ptype !=  FIL_PAGE_PAGE_COMPRESSED &&
> -               ptype != FIL_PAGE_PAGE_COMPRESSED_ENCRYPTED &&
> -               ptype != FIL_PAGE_COMPRESSED) {
> +       switch (ptype) {
> +       case FIL_PAGE_PAGE_COMPRESSED_ENCRYPTED:
> +               header_len = FIL_PAGE_DATA + FIL_PAGE_COMPRESSED_SIZE
> +                       + FIL_PAGE_COMPRESSION_METHOD_SIZE;
> +               break;
> +       case FIL_PAGE_PAGE_COMPRESSED:
> +               header_len = FIL_PAGE_DATA + FIL_PAGE_COMPRESSED_SIZE;
> +               break;
> +       default:
> +               /* The page is not in our format. */
>                 return;
>

This is also unrelated change but more like cosmetic, so ok.


>         }
>
>
> diff --git a/storage/innobase/include/buf0buf.ic
> b/storage/innobase/include/buf0buf.ic
> index f2b1b151598..5e75c446bbd 100644
> --- a/storage/innobase/include/buf0buf.ic
> +++ b/storage/innobase/include/buf0buf.ic
> @@ -734,9 +734,6 @@ buf_block_get_frame(
>         case BUF_BLOCK_ZIP_PAGE:
>         case BUF_BLOCK_ZIP_DIRTY:
>         case BUF_BLOCK_NOT_USED:
> -               if (block->page.encrypted) {
> -                       goto ok;
> -               }
>

Hmm, this is unrelated but could be actually correct.


>                 ut_error;
>                 break;
>         case BUF_BLOCK_FILE_PAGE:
>
>
> @@ -145,52 +137,22 @@ fil_page_type_validate(
>                 page_type == FIL_PAGE_TYPE_BLOB ||
>                 page_type == FIL_PAGE_TYPE_ZBLOB ||
>                 page_type == FIL_PAGE_TYPE_ZBLOB2 ||
> -               page_type == FIL_PAGE_COMPRESSED ||
> -               page_type == FIL_PAGE_TYPE_UNKNOWN ||
> -               page_type == FIL_PAGE_ENCRYPTED ||
> -               page_type == FIL_PAGE_COMPRESSED_AND_ENCRYPTED ||
> -               page_type == FIL_PAGE_ENCRYPTED_RTREE))) {
> -
> -               uint key_version = mach_read_from_4(page +
> FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION);
> -               bool page_compressed = (page_type ==
> FIL_PAGE_PAGE_COMPRESSED);
> -               bool page_compressed_encrypted = (page_type ==
> FIL_PAGE_PAGE_COMPRESSED_ENCRYPTED);
> +               page_type == FIL_PAGE_TYPE_UNKNOWN))) {
>

Why you change this code, it is not MySQL compression code ?


> +
>                 ulint space = mach_read_from_4(page +
> FIL_PAGE_ARCH_LOG_NO_OR_SPACE_ID);
>                 ulint offset = mach_read_from_4(page + FIL_PAGE_OFFSET);
> -               ib_uint64_t lsn = mach_read_from_8(page + FIL_PAGE_LSN);
> -               ulint compressed_len = mach_read_from_2(page +
> FIL_PAGE_DATA);
>                 fil_system_enter();
>                 fil_space_t* rspace = fil_space_get_by_id(space);
>                 fil_system_exit();
>
>                 /* Dump out the page info */
> -               fprintf(stderr, "InnoDB: Space %lu offset %lu name %s
> page_type %lu page_type_name %s\n"
> -                       "InnoDB: key_version %u page_compressed %d
> page_compressed_encrypted %d lsn %llu compressed_len %lu\n",
> -                       space, offset, rspace->name, page_type,
> fil_get_page_type_name(page_type),
> -                       key_version, page_compressed,
> page_compressed_encrypted, (ulonglong)lsn, compressed_len);
> -               fflush(stderr);
> -
> -               ut_ad(page_type == FIL_PAGE_PAGE_COMPRESSED ||
> -                       page_type == FIL_PAGE_PAGE_COMPRESSED_ENCRYPTED ||
> -                       page_type == FIL_PAGE_INDEX ||
> -                       page_type == FIL_PAGE_RTREE ||
> -                       page_type == FIL_PAGE_UNDO_LOG ||
> -                       page_type == FIL_PAGE_INODE ||
> -                       page_type == FIL_PAGE_IBUF_FREE_LIST ||
> -                       page_type == FIL_PAGE_TYPE_ALLOCATED ||
> -                       page_type == FIL_PAGE_IBUF_BITMAP ||
> -                       page_type == FIL_PAGE_TYPE_SYS ||
> -                       page_type == FIL_PAGE_TYPE_TRX_SYS ||
> -                       page_type == FIL_PAGE_TYPE_FSP_HDR ||
> -                       page_type == FIL_PAGE_TYPE_XDES ||
> -                       page_type == FIL_PAGE_TYPE_BLOB ||
> -                       page_type == FIL_PAGE_TYPE_ZBLOB ||
> -                       page_type == FIL_PAGE_TYPE_ZBLOB2 ||
> -                       page_type == FIL_PAGE_COMPRESSED ||
> -                       page_type == FIL_PAGE_TYPE_UNKNOWN ||
> -                       page_type == FIL_PAGE_ENCRYPTED ||
> -                       page_type == FIL_PAGE_COMPRESSED_AND_ENCRYPTED ||
> -                       page_type == FIL_PAGE_ENCRYPTED_RTREE);
> -
> +               ib::fatal() << "Page " << space << ":" << offset
> +                       << " name " << (rspace ? rspace->name : "???")
> +                       << " page_type " << page_type
> +                       << " key_version "
> +                       << mach_read_from_4(page +
> FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION)
> +                       << " lsn " << mach_read_from_8(page + FIL_PAGE_LSN)
> +                       << " compressed_len " << mach_read_from_2(page +
> FIL_PAGE_DATA);
>                 return false;
>         }
>
>
>
ok to push, rest of the changes.

R: Jan

Follow ups