From 41cd80fe065f3887db9f2d4f50729204ad13ba0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Lindstr=C3=B6m?= Date: Fri, 10 Feb 2017 16:05:37 +0200 Subject: [PATCH] After review fixes. --- storage/innobase/buf/buf0buf.cc | 34 +++++++++-------------- storage/innobase/buf/buf0dblwr.cc | 16 +++-------- storage/innobase/fil/fil0crypt.cc | 7 +++-- storage/innobase/include/buf0buf.h | 1 - storage/innobase/include/fil0fil.ic | 37 ++++++++++++------------- storage/xtradb/buf/buf0buf.cc | 42 +++++++++-------------------- storage/xtradb/buf/buf0dblwr.cc | 16 +++-------- storage/xtradb/fil/fil0crypt.cc | 7 +++-- storage/xtradb/include/buf0buf.h | 1 - storage/xtradb/include/fil0fil.ic | 37 ++++++++++++------------- 10 files changed, 76 insertions(+), 122 deletions(-) diff --git a/storage/innobase/buf/buf0buf.cc b/storage/innobase/buf/buf0buf.cc index 9d322d3c833..7263b8ce669 100644 --- a/storage/innobase/buf/buf0buf.cc +++ b/storage/innobase/buf/buf0buf.cc @@ -523,10 +523,11 @@ buf_page_is_checksum_valid_crc32( ("Page checksum crc32 not valid field1 " ULINTPF " field2 " ULINTPF " crc32 %u.", checksum_field1, checksum_field2, crc32)); + return (false); } - return(checksum_field1 == crc32 && checksum_field2 == crc32); + return (true); } /** Checks if the page is in innodb checksum format. @@ -634,7 +635,9 @@ buf_page_is_corrupted( compressed tables do not contain post compression checksum and FIL_PAGE_END_LSN_OLD_CHKSUM field stored. Note that space can be null if we are in fil_check_first_page() and first page - is not compressed or encrypted. */ + is not compressed or encrypted. Page checksum is verified + after decompression (i.e. normally pages are already + decompressed at this stage). */ if ((page_type == FIL_PAGE_PAGE_COMPRESSED || page_type == FIL_PAGE_PAGE_COMPRESSED_ENCRYPTED) && space && FSP_FLAGS_HAS_PAGE_COMPRESSION(space->flags)) { @@ -1111,7 +1114,6 @@ buf_block_init( block->page.state = BUF_BLOCK_NOT_USED; block->page.buf_fix_count = 0; block->page.io_fix = BUF_IO_NONE; - block->page.corrupted = false; block->page.key_version = 0; block->page.encrypted = false; block->page.real_size = 0; @@ -3773,7 +3775,6 @@ buf_page_init_low( bpage->write_size = 0; bpage->key_version = 0; bpage->encrypted = false; - bpage->corrupted = false; bpage->real_size = 0; bpage->slot = NULL; @@ -4478,8 +4479,7 @@ buf_page_check_corrupt( ulint zip_size = buf_page_get_zip_size(bpage); byte* dst_frame = (zip_size) ? bpage->zip.data : ((buf_block_t*) bpage)->frame; - ulint space_id = mach_read_from_4( - dst_frame + FIL_PAGE_ARCH_LOG_NO_OR_SPACE_ID); + ulint space_id = bpage->space; fil_space_t* space = fil_space_found_by_id(space_id); fil_space_crypt_t* crypt_data = space->crypt_data; bool still_encrypted = false; @@ -4510,9 +4510,8 @@ buf_page_check_corrupt( /* Pages that we think are unencrypted but do not match the checksum checks could be corrupted or encrypted or both. */ if (corrupted && !bpage->encrypted) { - bpage->corrupted = true; ib_logf(IB_LOG_LEVEL_ERROR, - "%s: Block in space_id %lu in file %s corrupted.", + "%s: Block in space_id " ULINTPF " in file %s corrupted.", page_type == FIL_PAGE_PAGE_COMPRESSED_ENCRYPTED ? "Maybe corruption" : "Corruption", space_id, space ? space->name : "NULL"); ib_logf(IB_LOG_LEVEL_ERROR, @@ -4520,11 +4519,10 @@ buf_page_check_corrupt( fil_get_page_type_name(page_type), page_type); } else if (still_encrypted || (bpage->encrypted && corrupted)) { bpage->encrypted = true; - bpage->corrupted = false; corrupted = true; ib_logf(IB_LOG_LEVEL_ERROR, - "Block in space_id %lu in file %s encrypted.", + "Block in space_id " ULINTPF " in file %s encrypted.", space_id, space ? space->name : "NULL"); ib_logf(IB_LOG_LEVEL_ERROR, "However key management plugin or used key_version %u is not found or" @@ -4635,7 +4633,7 @@ buf_page_io_complete( fprintf(stderr, " InnoDB: Error: space id and page n:o" " stored in the page\n" - "InnoDB: read in are %lu:%lu," + "InnoDB: read in are " ULINTPF ":" ULINTPF "," " should be %u:%u!\n", read_space_id, read_page_no, @@ -4745,13 +4743,6 @@ database_corrupted: && fil_page_get_type(frame) == FIL_PAGE_INDEX && page_is_leaf(frame)) { - buf_block_t* block; - ibool update_ibuf_bitmap; - - block = (buf_block_t *) bpage; - - update_ibuf_bitmap = TRUE; - if (bpage && bpage->encrypted) { ib_logf(IB_LOG_LEVEL_WARN, "Table in tablespace %lu encrypted." @@ -4760,10 +4751,11 @@ database_corrupted: " Can't continue opening the table.\n", (ulint)bpage->space, bpage->key_version); } else { + ibuf_merge_or_delete_for_page( - block, bpage->space, + (buf_block_t*)bpage, bpage->space, bpage->offset, buf_page_get_zip_size(bpage), - update_ibuf_bitmap); + TRUE); } } @@ -6242,7 +6234,7 @@ buf_page_decrypt_after_read( /* Page is encrypted if encryption information is found from tablespace and page contains used key_version. This is true also for pages first compressed and then encrypted. */ - if (!crypt_data && key_version != 0) { + if (!crypt_data) { key_version = 0; } diff --git a/storage/innobase/buf/buf0dblwr.cc b/storage/innobase/buf/buf0dblwr.cc index 02980c80112..75a85de4313 100644 --- a/storage/innobase/buf/buf0dblwr.cc +++ b/storage/innobase/buf/buf0dblwr.cc @@ -382,14 +382,7 @@ buf_dblwr_init_or_load_pages( doublewrite = read_buf + TRX_SYS_DOUBLEWRITE; - if (mach_read_from_4(read_buf + FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION) != 0) { - bool decrypted = false; - byte* tmp = fil_space_decrypt((ulint)TRX_SYS_SPACE, - read_buf + UNIV_PAGE_SIZE, - UNIV_PAGE_SIZE, /* page size */ - read_buf, &decrypted); - doublewrite = tmp + TRX_SYS_DOUBLEWRITE; - } + /* TRX_SYS_PAGE_NO is not encrypted see fil_crypt_rotate_page() */ if (mach_read_from_4(doublewrite + TRX_SYS_DOUBLEWRITE_MAGIC) == TRX_SYS_DOUBLEWRITE_MAGIC_N) { @@ -488,7 +481,6 @@ buf_dblwr_process() byte* read_buf; byte* unaligned_read_buf; recv_dblwr_t& recv_dblwr = recv_sys->dblwr; - fil_space_t* space=NULL; unaligned_read_buf = static_cast(ut_malloc(2 * UNIV_PAGE_SIZE)); @@ -497,6 +489,7 @@ buf_dblwr_process() for (std::list::iterator i = recv_dblwr.pages.begin(); i != recv_dblwr.pages.end(); ++i, ++page_no_dblwr ) { + fil_space_t* space=NULL; page = *i; page_no = mach_read_from_4(page + FIL_PAGE_OFFSET); space_id = mach_read_from_4(page + FIL_PAGE_SPACE_ID); @@ -516,10 +509,7 @@ buf_dblwr_process() continue; } - if (!space) { - space = fil_space_found_by_id(space_id); - } - + space = fil_space_found_by_id(space_id); ulint zip_size = fil_space_get_zip_size(space_id); ut_ad(!buf_page_is_zeroes(page, zip_size)); diff --git a/storage/innobase/fil/fil0crypt.cc b/storage/innobase/fil/fil0crypt.cc index 13f2e905bc4..5d5490fc43f 100644 --- a/storage/innobase/fil/fil0crypt.cc +++ b/storage/innobase/fil/fil0crypt.cc @@ -970,8 +970,10 @@ fil_space_verify_crypt_checksum( return false; } + srv_checksum_algorithm_t algorithm = + static_cast(srv_checksum_algorithm); /* If no checksum is used, can't continue checking. */ - if (srv_checksum_algorithm == SRV_CHECKSUM_ALGORITHM_NONE) { + if (algorithm == SRV_CHECKSUM_ALGORITHM_NONE) { return(true); } @@ -987,7 +989,8 @@ fil_space_verify_crypt_checksum( } /* Compressed and encrypted pages do not have checksum. Assume not - corrupted. */ + corrupted. Page verification happens after decompression in + buf_page_io_complete() using buf_page_is_corrupted(). */ if (mach_read_from_2(page+FIL_PAGE_TYPE) == FIL_PAGE_PAGE_COMPRESSED_ENCRYPTED) { return (true); } diff --git a/storage/innobase/include/buf0buf.h b/storage/innobase/include/buf0buf.h index a0109b4aa64..8568d8b04f2 100644 --- a/storage/innobase/include/buf0buf.h +++ b/storage/innobase/include/buf0buf.h @@ -1644,7 +1644,6 @@ struct buf_page_t{ unsigned key_version; /*!< key version for this block */ bool encrypted; /*!< page is still encrypted */ - bool corrupted; /*!< page is corrupted */ ulint real_size; /*!< Real size of the page Normal pages == UNIV_PAGE_SIZE diff --git a/storage/innobase/include/fil0fil.ic b/storage/innobase/include/fil0fil.ic index 772255bb75b..bea274d2c86 100644 --- a/storage/innobase/include/fil0fil.ic +++ b/storage/innobase/include/fil0fil.ic @@ -58,43 +58,40 @@ fil_get_page_type_name( { switch(page_type) { case FIL_PAGE_PAGE_COMPRESSED: - return (const char*)"PAGE_COMPRESSED"; + return "PAGE_COMPRESSED"; case FIL_PAGE_PAGE_COMPRESSED_ENCRYPTED: - return (const char*)"PAGE_COMPRESSED_ENCRYPTED"; + return "PAGE_COMPRESSED_ENCRYPTED"; case FIL_PAGE_INDEX: - return (const char*)"INDEX"; + return "INDEX"; case FIL_PAGE_UNDO_LOG: - return (const char*)"UNDO LOG"; + return "UNDO LOG"; case FIL_PAGE_INODE: - return (const char*)"INODE"; + return "INODE"; case FIL_PAGE_IBUF_FREE_LIST: - return (const char*)"IBUF_FREE_LIST"; + return "IBUF_FREE_LIST"; case FIL_PAGE_TYPE_ALLOCATED: - return (const char*)"ALLOCATED"; + return "ALLOCATED"; case FIL_PAGE_IBUF_BITMAP: - return (const char*)"IBUF_BITMAP"; + return "IBUF_BITMAP"; case FIL_PAGE_TYPE_SYS: - return (const char*)"SYS"; + return "SYS"; case FIL_PAGE_TYPE_TRX_SYS: - return (const char*)"TRX_SYS"; + return "TRX_SYS"; case FIL_PAGE_TYPE_FSP_HDR: - return (const char*)"FSP_HDR"; + return "FSP_HDR"; case FIL_PAGE_TYPE_XDES: - return (const char*)"XDES"; + return "XDES"; case FIL_PAGE_TYPE_BLOB: - return (const char*)"BLOB"; + return "BLOB"; case FIL_PAGE_TYPE_ZBLOB: - return (const char*)"ZBLOB"; + return "ZBLOB"; case FIL_PAGE_TYPE_ZBLOB2: - return (const char*)"ZBLOB2"; + return "ZBLOB2"; case FIL_PAGE_TYPE_COMPRESSED: - return (const char*)"ORACLE PAGE COMPRESSED"; - - /* No default to make compiler generate warning if - new page type is added but not handled here. */ + return "ORACLE PAGE COMPRESSED"; } - return (const char*)"PAGE TYPE CORRUPTED"; + return "PAGE TYPE CORRUPTED"; } diff --git a/storage/xtradb/buf/buf0buf.cc b/storage/xtradb/buf/buf0buf.cc index e1746240ef5..4ed7dab052b 100644 --- a/storage/xtradb/buf/buf0buf.cc +++ b/storage/xtradb/buf/buf0buf.cc @@ -588,9 +588,10 @@ buf_page_is_checksum_valid_crc32( ("Page checksum crc32 not valid field1 " ULINTPF " field2 " ULINTPF " crc32 %u.", checksum_field1, checksum_field2, crc32)); + return (false); } - return(checksum_field1 == crc32 && checksum_field2 == crc32); + return (true); } /** Checks if the page is in innodb checksum format. @@ -700,7 +701,9 @@ buf_page_is_corrupted( compressed tables do not contain post compression checksum and FIL_PAGE_END_LSN_OLD_CHKSUM field stored. Note that space can be null if we are in fil_check_first_page() and first page - is not compressed or encrypted. */ + is not compressed or encrypted. Page checksum is verified + after decompression (i.e. normally pages are already + decompressed at this stage). */ if ((page_type == FIL_PAGE_PAGE_COMPRESSED || page_type == FIL_PAGE_PAGE_COMPRESSED_ENCRYPTED) && space && FSP_FLAGS_HAS_PAGE_COMPRESSION(space->flags)) { @@ -1178,7 +1181,6 @@ buf_block_init( block->page.buf_fix_count = 0; block->page.io_fix = BUF_IO_NONE; block->page.encrypted = false; - block->page.corrupted = false; block->page.key_version = 0; block->page.real_size = 0; block->page.write_size = 0; @@ -3854,7 +3856,6 @@ buf_page_init_low( bpage->oldest_modification = 0; bpage->write_size = 0; bpage->encrypted = false; - bpage->corrupted = false; bpage->key_version = 0; bpage->real_size = 0; @@ -4578,8 +4579,7 @@ buf_page_check_corrupt( ulint zip_size = buf_page_get_zip_size(bpage); byte* dst_frame = (zip_size) ? bpage->zip.data : ((buf_block_t*) bpage)->frame; - ulint space_id = mach_read_from_4( - dst_frame + FIL_PAGE_ARCH_LOG_NO_OR_SPACE_ID); + ulint space_id = bpage->space; fil_space_t* space = fil_space_found_by_id(space_id); fil_space_crypt_t* crypt_data = space->crypt_data; bool still_encrypted = false; @@ -4611,9 +4611,8 @@ buf_page_check_corrupt( /* Pages that we think are unencrypted but do not match the checksum checks could be corrupted or encrypted or both. */ if (corrupted && !bpage->encrypted) { - bpage->corrupted = true; ib_logf(IB_LOG_LEVEL_ERROR, - "%s: Block in space_id %lu in file %s corrupted.", + "%s: Block in space_id " ULINTPF " in file %s corrupted.", page_type == FIL_PAGE_PAGE_COMPRESSED_ENCRYPTED ? "Maybe corruption" : "Corruption", space_id, space ? space->name : "NULL"); ib_logf(IB_LOG_LEVEL_ERROR, @@ -4621,11 +4620,10 @@ buf_page_check_corrupt( fil_get_page_type_name(page_type), page_type); } else if (still_encrypted || (bpage->encrypted && corrupted)) { bpage->encrypted = true; - bpage->corrupted = false; corrupted = true; ib_logf(IB_LOG_LEVEL_ERROR, - "Block in space_id %lu in file %s encrypted.", + "Block in space_id " ULINTPF " in file %s encrypted.", space_id, space ? space->name : "NULL"); ib_logf(IB_LOG_LEVEL_ERROR, "However key management plugin or used key_version %u is not found or" @@ -4698,7 +4696,6 @@ buf_page_io_complete( bpage->offset, bpage->space); corrupted = true; - bpage->corrupted = true; goto database_corrupted; } @@ -4738,7 +4735,7 @@ buf_page_io_complete( fprintf(stderr, " InnoDB: Error: space id and page n:o" " stored in the page\n" - "InnoDB: read in are %lu:%lu," + "InnoDB: read in are " ULINTPF ":" ULINTPF "," " should be %u:%u!\n", read_space_id, read_page_no, @@ -4872,20 +4869,6 @@ database_corrupted: && fil_page_get_type(frame) == FIL_PAGE_INDEX && page_is_leaf(frame)) { - buf_block_t* block; - ibool update_ibuf_bitmap; - - if (UNIV_UNLIKELY(bpage->is_corrupt && - srv_pass_corrupt_table)) { - - block = NULL; - update_ibuf_bitmap = FALSE; - } else { - - block = (buf_block_t *) bpage; - update_ibuf_bitmap = TRUE; - } - if (bpage && bpage->encrypted) { ib_logf(IB_LOG_LEVEL_WARN, "Table in tablespace %lu encrypted." @@ -4894,10 +4877,11 @@ database_corrupted: " Can't continue opening the table.\n", (ulint)bpage->space, bpage->key_version); } else { + ibuf_merge_or_delete_for_page( - block, bpage->space, + (buf_block_t*)bpage, bpage->space, bpage->offset, buf_page_get_zip_size(bpage), - update_ibuf_bitmap); + TRUE); } } @@ -6418,7 +6402,7 @@ buf_page_decrypt_after_read( /* Page is encrypted if encryption information is found from tablespace and page contains used key_version. This is true also for pages first compressed and then encrypted. */ - if (!crypt_data && key_version != 0) { + if (!crypt_data) { key_version = 0; } diff --git a/storage/xtradb/buf/buf0dblwr.cc b/storage/xtradb/buf/buf0dblwr.cc index a60d3f2ed12..5a49ccd571f 100644 --- a/storage/xtradb/buf/buf0dblwr.cc +++ b/storage/xtradb/buf/buf0dblwr.cc @@ -382,14 +382,7 @@ buf_dblwr_init_or_load_pages( doublewrite = read_buf + TRX_SYS_DOUBLEWRITE; - if (mach_read_from_4(read_buf + FIL_PAGE_FILE_FLUSH_LSN_OR_KEY_VERSION) != 0) { - bool decrypted = false; - byte* tmp = fil_space_decrypt((ulint)TRX_SYS_SPACE, - read_buf + UNIV_PAGE_SIZE, - UNIV_PAGE_SIZE, /* page size */ - read_buf, &decrypted); - doublewrite = tmp + TRX_SYS_DOUBLEWRITE; - } + /* TRX_SYS_PAGE_NO is not encrypted see fil_crypt_rotate_page() */ if (mach_read_from_4(doublewrite + TRX_SYS_DOUBLEWRITE_MAGIC) == TRX_SYS_DOUBLEWRITE_MAGIC_N) { @@ -488,7 +481,6 @@ buf_dblwr_process() byte* read_buf; byte* unaligned_read_buf; recv_dblwr_t& recv_dblwr = recv_sys->dblwr; - fil_space_t* space=NULL; unaligned_read_buf = static_cast(ut_malloc(2 * UNIV_PAGE_SIZE)); @@ -497,6 +489,7 @@ buf_dblwr_process() for (std::list::iterator i = recv_dblwr.pages.begin(); i != recv_dblwr.pages.end(); ++i, ++page_no_dblwr ) { + fil_space_t* space=NULL; page = *i; page_no = mach_read_from_4(page + FIL_PAGE_OFFSET); space_id = mach_read_from_4(page + FIL_PAGE_SPACE_ID); @@ -516,10 +509,7 @@ buf_dblwr_process() continue; } - if (!space) { - space = fil_space_found_by_id(space_id); - } - + space = fil_space_found_by_id(space_id); ulint zip_size = fil_space_get_zip_size(space_id); ut_ad(!buf_page_is_zeroes(page, zip_size)); diff --git a/storage/xtradb/fil/fil0crypt.cc b/storage/xtradb/fil/fil0crypt.cc index f4a27afb969..251ace81e15 100644 --- a/storage/xtradb/fil/fil0crypt.cc +++ b/storage/xtradb/fil/fil0crypt.cc @@ -970,8 +970,10 @@ fil_space_verify_crypt_checksum( return false; } + srv_checksum_algorithm_t algorithm = + static_cast(srv_checksum_algorithm); /* If no checksum is used, can't continue checking. */ - if (srv_checksum_algorithm == SRV_CHECKSUM_ALGORITHM_NONE) { + if (algorithm == SRV_CHECKSUM_ALGORITHM_NONE) { return(true); } @@ -987,7 +989,8 @@ fil_space_verify_crypt_checksum( } /* Compressed and encrypted pages do not have checksum. Assume not - corrupted. */ + corrupted. Page verification happens after decompression in + buf_page_io_complete() using buf_page_is_corrupted(). */ if (mach_read_from_2(page+FIL_PAGE_TYPE) == FIL_PAGE_PAGE_COMPRESSED_ENCRYPTED) { return (true); } diff --git a/storage/xtradb/include/buf0buf.h b/storage/xtradb/include/buf0buf.h index 7a39d544a24..2548b0daeb2 100644 --- a/storage/xtradb/include/buf0buf.h +++ b/storage/xtradb/include/buf0buf.h @@ -1681,7 +1681,6 @@ struct buf_page_t{ unsigned key_version; /*!< key version for this block */ bool encrypted; /*!< page is still encrypted */ - bool corrupted; /*!< page is corrupted */ ulint real_size; /*!< Real size of the page Normal pages == UNIV_PAGE_SIZE diff --git a/storage/xtradb/include/fil0fil.ic b/storage/xtradb/include/fil0fil.ic index 2b3ddc04b3b..1179eea8b8e 100644 --- a/storage/xtradb/include/fil0fil.ic +++ b/storage/xtradb/include/fil0fil.ic @@ -58,43 +58,40 @@ fil_get_page_type_name( { switch(page_type) { case FIL_PAGE_PAGE_COMPRESSED: - return (const char*)"PAGE_COMPRESSED"; + return "PAGE_COMPRESSED"; case FIL_PAGE_PAGE_COMPRESSED_ENCRYPTED: - return (const char*)"PAGE_COMPRESSED_ENCRYPTED"; + return "PAGE_COMPRESSED_ENCRYPTED"; case FIL_PAGE_INDEX: - return (const char*)"INDEX"; + return "INDEX"; case FIL_PAGE_UNDO_LOG: - return (const char*)"UNDO LOG"; + return "UNDO LOG"; case FIL_PAGE_INODE: - return (const char*)"INODE"; + return "INODE"; case FIL_PAGE_IBUF_FREE_LIST: - return (const char*)"IBUF_FREE_LIST"; + return "IBUF_FREE_LIST"; case FIL_PAGE_TYPE_ALLOCATED: - return (const char*)"ALLOCATED"; + return "ALLOCATED"; case FIL_PAGE_IBUF_BITMAP: - return (const char*)"IBUF_BITMAP"; + return "IBUF_BITMAP"; case FIL_PAGE_TYPE_SYS: - return (const char*)"SYS"; + return "SYS"; case FIL_PAGE_TYPE_TRX_SYS: - return (const char*)"TRX_SYS"; + return "TRX_SYS"; case FIL_PAGE_TYPE_FSP_HDR: - return (const char*)"FSP_HDR"; + return "FSP_HDR"; case FIL_PAGE_TYPE_XDES: - return (const char*)"XDES"; + return "XDES"; case FIL_PAGE_TYPE_BLOB: - return (const char*)"BLOB"; + return "BLOB"; case FIL_PAGE_TYPE_ZBLOB: - return (const char*)"ZBLOB"; + return "ZBLOB"; case FIL_PAGE_TYPE_ZBLOB2: - return (const char*)"ZBLOB2"; + return "ZBLOB2"; case FIL_PAGE_TYPE_COMPRESSED: - return (const char*)"ORACLE PAGE COMPRESSED"; - - /* No default to make compiler generate warning if - new page type is added but not handled here. */ + return "ORACLE PAGE COMPRESSED"; } - return (const char*)"PAGE TYPE CORRUPTED"; + return "PAGE TYPE CORRUPTED"; }