From a80af35a85d329f3ac1456b5931ffc541c8e54c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 20 Mar 2018 13:03:01 +0200 Subject: [PATCH] MDEV-12396 IMPORT cleanup: ROW_FORMAT=COMPRESSED Initialize block.page.zip only once. PageConverter::update(): Initialize m_page_zip_ptr as late as possible. (We should really remove it at some point.) PageConverter::operator(): Refer to block->page.zip instead of m_page_zip_ptr. AbstractCallback::get_frame(): Define static. Refer to block->page.zip.data directly. fil_iterate(): Refer to block->page.zip.data directly. fil_tablespace_iterate(): Initialize block.page.zip.data as soon as possible. --- storage/innobase/row/row0import.cc | 120 ++++++++++++++--------------- storage/xtradb/row/row0import.cc | 120 ++++++++++++++--------------- 2 files changed, 116 insertions(+), 124 deletions(-) diff --git a/storage/innobase/row/row0import.cc b/storage/innobase/row/row0import.cc index 24dc3a66f1f..5977bd1a77b 100644 --- a/storage/innobase/row/row0import.cc +++ b/storage/innobase/row/row0import.cc @@ -441,20 +441,17 @@ public: bool is_interrupted() const { return trx_is_interrupted(m_trx); } -protected: /** Get the data page depending on the table type, compressed or not. @param block - block read from disk @retval the buffer frame */ - buf_frame_t* get_frame(buf_block_t* block) const UNIV_NOTHROW + static byte* get_frame(const buf_block_t* block) { - if (is_compressed_table()) { - return(block->page.zip.data); - } - - return(buf_block_get_frame(block)); + return block->page.zip.data + ? block->page.zip.data : block->frame; } +protected: /** Get the physical offset of the extent descriptor within the page. @param page_no - page number of the extent descriptor @@ -1981,6 +1978,14 @@ PageConverter::update_page( { dberr_t err = DB_SUCCESS; + ut_ad(!block->page.zip.data == !is_compressed_table()); + + if (block->page.zip.data) { + m_page_zip_ptr = &block->page.zip; + } else { + ut_ad(!m_page_zip_ptr); + } + switch (page_type = fil_page_get_type(get_frame(block))) { case FIL_PAGE_TYPE_FSP_HDR: /* Work directly on the uncompressed page headers. */ @@ -2043,46 +2048,38 @@ updated then its state must be set to BUF_PAGE_NOT_USED. dberr_t PageConverter::operator() (os_offset_t, buf_block_t* block) UNIV_NOTHROW { - ulint page_type; - - if (is_compressed_table()) { - m_page_zip_ptr = &block->page.zip; - } else { - ut_ad(m_page_zip_ptr == 0); - } - - dberr_t err = update_page(block, page_type); - if (err == DB_SUCCESS) { - /* Note: For compressed pages this function will write to the - zip descriptor and for uncompressed pages it will write to - page (ie. the block->frame). Therefore the caller should write - out the descriptor contents and not block->frame for compressed - pages. */ - - if (!is_compressed_table() || page_type == FIL_PAGE_INDEX) { - - buf_flush_init_for_writing( - !is_compressed_table() - ? block->frame : block->page.zip.data, - !is_compressed_table() ? 0 : m_page_zip_ptr, - m_current_lsn); - } else { - /* Calculate and update the checksum of non-btree - pages for compressed tables explicitly here. */ - - buf_flush_update_zip_checksum( - get_frame(block), get_zip_size(), - m_current_lsn); - } - } - /* If we already had an old page with matching number in the buffer pool, evict it now, because we no longer evict the pages on DISCARD TABLESPACE. */ buf_page_get_gen(get_space_id(), get_zip_size(), block->page.offset, RW_NO_LATCH, NULL, BUF_EVICT_IF_IN_POOL, __FILE__, __LINE__, NULL); - return(err); + + ulint page_type; + + dberr_t err = update_page(block, page_type); + if (err != DB_SUCCESS) return err; + + /* Note: For compressed pages this function will write to the + zip descriptor and for uncompressed pages it will write to + page (ie. the block->frame). Therefore the caller should write + out the descriptor contents and not block->frame for compressed + pages. */ + + if (!is_compressed_table() || page_type == FIL_PAGE_INDEX) { + buf_flush_init_for_writing( + get_frame(block), + block->page.zip.data ? &block->page.zip : NULL, + m_current_lsn); + } else { + /* Calculate and update the checksum of non-btree + pages for compressed tables explicitly here. */ + buf_flush_update_zip_checksum( + get_frame(block), get_zip_size(), + m_current_lsn); + } + + return DB_SUCCESS; } /*****************************************************************//** @@ -3396,10 +3393,9 @@ fil_iterate( ut_ad(!srv_read_only_mode); - /* TODO: For compressed tables we do a lot of useless + /* TODO: For ROW_FORMAT=COMPRESSED tables we do a lot of useless copying for non-index pages. Unfortunately, it is required by buf_zip_decompress() */ - const bool row_compressed = callback.get_zip_size() > 0; for (offset = iter.start; offset < iter.end; offset += n_bytes) { if (callback.is_interrupted()) { @@ -3407,18 +3403,12 @@ fil_iterate( } byte* io_buffer = iter.io_buffer; - block->frame = io_buffer; - if (row_compressed) { - page_zip_des_init(&block->page.zip); - page_zip_set_size(&block->page.zip, iter.page_size); - block->page.zip.data = block->frame + UNIV_PAGE_SIZE; - ut_d(block->page.zip.m_external = true); - ut_ad(iter.page_size == callback.get_zip_size()); - + if (block->page.zip.data) { /* Zip IO is done in the compressed page buffer. */ io_buffer = block->page.zip.data; + ut_ad(PAGE_ZIP_MATCH(block->frame, &block->page.zip)); } /* We have to read the exact number of bytes. Otherwise the @@ -3493,7 +3483,8 @@ fil_iterate( if (decrypted) { updated = true; } else { - if (!page_compressed && !row_compressed) { + if (!page_compressed + && !block->page.zip.data) { block->frame = src; frame_changed = true; } else { @@ -3559,8 +3550,7 @@ page_corrupted: if (encrypted) { memcpy(writeptr + (i * size), - row_compressed ? block->page.zip.data : - block->frame, size); + callback.get_frame(block), size); } if (frame_changed) { @@ -3728,6 +3718,13 @@ fil_tablespace_iterate( err = DB_IO_ERROR; } else if ((err = callback.init(file_size, &block)) == DB_SUCCESS) { + if (const ulint zip_size = callback.get_zip_size()) { + page_zip_set_size(&block.page.zip, zip_size); + /* ROW_FORMAT=COMPRESSED is not optimised for block IO + for now. We do the IMPORT page by page. */ + n_io_buffers = 1; + } + fil_iterator_t iter; iter.file = file; @@ -3748,14 +3745,6 @@ fil_tablespace_iterate( iter.crypt_data = fil_space_read_crypt_data( 0, page, crypt_data_offset); - /* Compressed pages can't be optimised for block IO for now. - We do the IMPORT page by page. */ - - if (callback.get_zip_size() > 0) { - iter.n_io_buffers = 1; - ut_a(iter.page_size == callback.get_zip_size()); - } - /** If tablespace is encrypted, it needs extra buffers */ if (iter.crypt_data != NULL) { /* decrease io buffers so that memory @@ -3780,6 +3769,13 @@ fil_tablespace_iterate( ut_align(crypt_io_buffer, UNIV_PAGE_SIZE)); } + if (block.page.zip.ssize) { + ut_ad(iter.n_io_buffers == 1); + block.frame = iter.io_buffer; + block.page.zip.data = block.frame + UNIV_PAGE_SIZE; + ut_d(block.page.zip.m_external = true); + } + err = fil_iterate(iter, &block, callback); mem_free(io_buffer); diff --git a/storage/xtradb/row/row0import.cc b/storage/xtradb/row/row0import.cc index 24dc3a66f1f..5977bd1a77b 100644 --- a/storage/xtradb/row/row0import.cc +++ b/storage/xtradb/row/row0import.cc @@ -441,20 +441,17 @@ public: bool is_interrupted() const { return trx_is_interrupted(m_trx); } -protected: /** Get the data page depending on the table type, compressed or not. @param block - block read from disk @retval the buffer frame */ - buf_frame_t* get_frame(buf_block_t* block) const UNIV_NOTHROW + static byte* get_frame(const buf_block_t* block) { - if (is_compressed_table()) { - return(block->page.zip.data); - } - - return(buf_block_get_frame(block)); + return block->page.zip.data + ? block->page.zip.data : block->frame; } +protected: /** Get the physical offset of the extent descriptor within the page. @param page_no - page number of the extent descriptor @@ -1981,6 +1978,14 @@ PageConverter::update_page( { dberr_t err = DB_SUCCESS; + ut_ad(!block->page.zip.data == !is_compressed_table()); + + if (block->page.zip.data) { + m_page_zip_ptr = &block->page.zip; + } else { + ut_ad(!m_page_zip_ptr); + } + switch (page_type = fil_page_get_type(get_frame(block))) { case FIL_PAGE_TYPE_FSP_HDR: /* Work directly on the uncompressed page headers. */ @@ -2043,46 +2048,38 @@ updated then its state must be set to BUF_PAGE_NOT_USED. dberr_t PageConverter::operator() (os_offset_t, buf_block_t* block) UNIV_NOTHROW { - ulint page_type; - - if (is_compressed_table()) { - m_page_zip_ptr = &block->page.zip; - } else { - ut_ad(m_page_zip_ptr == 0); - } - - dberr_t err = update_page(block, page_type); - if (err == DB_SUCCESS) { - /* Note: For compressed pages this function will write to the - zip descriptor and for uncompressed pages it will write to - page (ie. the block->frame). Therefore the caller should write - out the descriptor contents and not block->frame for compressed - pages. */ - - if (!is_compressed_table() || page_type == FIL_PAGE_INDEX) { - - buf_flush_init_for_writing( - !is_compressed_table() - ? block->frame : block->page.zip.data, - !is_compressed_table() ? 0 : m_page_zip_ptr, - m_current_lsn); - } else { - /* Calculate and update the checksum of non-btree - pages for compressed tables explicitly here. */ - - buf_flush_update_zip_checksum( - get_frame(block), get_zip_size(), - m_current_lsn); - } - } - /* If we already had an old page with matching number in the buffer pool, evict it now, because we no longer evict the pages on DISCARD TABLESPACE. */ buf_page_get_gen(get_space_id(), get_zip_size(), block->page.offset, RW_NO_LATCH, NULL, BUF_EVICT_IF_IN_POOL, __FILE__, __LINE__, NULL); - return(err); + + ulint page_type; + + dberr_t err = update_page(block, page_type); + if (err != DB_SUCCESS) return err; + + /* Note: For compressed pages this function will write to the + zip descriptor and for uncompressed pages it will write to + page (ie. the block->frame). Therefore the caller should write + out the descriptor contents and not block->frame for compressed + pages. */ + + if (!is_compressed_table() || page_type == FIL_PAGE_INDEX) { + buf_flush_init_for_writing( + get_frame(block), + block->page.zip.data ? &block->page.zip : NULL, + m_current_lsn); + } else { + /* Calculate and update the checksum of non-btree + pages for compressed tables explicitly here. */ + buf_flush_update_zip_checksum( + get_frame(block), get_zip_size(), + m_current_lsn); + } + + return DB_SUCCESS; } /*****************************************************************//** @@ -3396,10 +3393,9 @@ fil_iterate( ut_ad(!srv_read_only_mode); - /* TODO: For compressed tables we do a lot of useless + /* TODO: For ROW_FORMAT=COMPRESSED tables we do a lot of useless copying for non-index pages. Unfortunately, it is required by buf_zip_decompress() */ - const bool row_compressed = callback.get_zip_size() > 0; for (offset = iter.start; offset < iter.end; offset += n_bytes) { if (callback.is_interrupted()) { @@ -3407,18 +3403,12 @@ fil_iterate( } byte* io_buffer = iter.io_buffer; - block->frame = io_buffer; - if (row_compressed) { - page_zip_des_init(&block->page.zip); - page_zip_set_size(&block->page.zip, iter.page_size); - block->page.zip.data = block->frame + UNIV_PAGE_SIZE; - ut_d(block->page.zip.m_external = true); - ut_ad(iter.page_size == callback.get_zip_size()); - + if (block->page.zip.data) { /* Zip IO is done in the compressed page buffer. */ io_buffer = block->page.zip.data; + ut_ad(PAGE_ZIP_MATCH(block->frame, &block->page.zip)); } /* We have to read the exact number of bytes. Otherwise the @@ -3493,7 +3483,8 @@ fil_iterate( if (decrypted) { updated = true; } else { - if (!page_compressed && !row_compressed) { + if (!page_compressed + && !block->page.zip.data) { block->frame = src; frame_changed = true; } else { @@ -3559,8 +3550,7 @@ page_corrupted: if (encrypted) { memcpy(writeptr + (i * size), - row_compressed ? block->page.zip.data : - block->frame, size); + callback.get_frame(block), size); } if (frame_changed) { @@ -3728,6 +3718,13 @@ fil_tablespace_iterate( err = DB_IO_ERROR; } else if ((err = callback.init(file_size, &block)) == DB_SUCCESS) { + if (const ulint zip_size = callback.get_zip_size()) { + page_zip_set_size(&block.page.zip, zip_size); + /* ROW_FORMAT=COMPRESSED is not optimised for block IO + for now. We do the IMPORT page by page. */ + n_io_buffers = 1; + } + fil_iterator_t iter; iter.file = file; @@ -3748,14 +3745,6 @@ fil_tablespace_iterate( iter.crypt_data = fil_space_read_crypt_data( 0, page, crypt_data_offset); - /* Compressed pages can't be optimised for block IO for now. - We do the IMPORT page by page. */ - - if (callback.get_zip_size() > 0) { - iter.n_io_buffers = 1; - ut_a(iter.page_size == callback.get_zip_size()); - } - /** If tablespace is encrypted, it needs extra buffers */ if (iter.crypt_data != NULL) { /* decrease io buffers so that memory @@ -3780,6 +3769,13 @@ fil_tablespace_iterate( ut_align(crypt_io_buffer, UNIV_PAGE_SIZE)); } + if (block.page.zip.ssize) { + ut_ad(iter.n_io_buffers == 1); + block.frame = iter.io_buffer; + block.page.zip.data = block.frame + UNIV_PAGE_SIZE; + ut_d(block.page.zip.m_external = true); + } + err = fil_iterate(iter, &block, callback); mem_free(io_buffer);