Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The entry.tag is incorrect after full gc #5093

Closed
JaySon-Huang opened this issue Jun 9, 2022 · 0 comments · Fixed by #5094
Closed

The entry.tag is incorrect after full gc #5093

JaySon-Huang opened this issue Jun 9, 2022 · 0 comments · Fixed by #5094
Labels
affects-6.1 This bug affects the 6.1.x(LTS) versions. component/storage severity/major type/bug The issue is confirmed as a bug.

Comments

@JaySon-Huang
Copy link
Contributor

JaySon-Huang commented Jun 9, 2022

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

TEST_F(PageStorageTest, EntryTagAfterFullGC)
try
{
    {
        PageStorage::Config config;
        config.blob_heavy_gc_valid_rate = 1.0; /// always run full gc
        page_storage = reopenWithConfig(config);
    }

    const size_t buf_sz = 1024;
    char c_buff[buf_sz];

    for (size_t i = 0; i < buf_sz; ++i)
    {
        c_buff[i] = i % 0xff;
    }

    PageId page_id = 120;
    UInt64 tag = 12345;
    {
        WriteBatch batch;
        batch.putPage(page_id, tag, std::make_shared<ReadBufferFromMemory>(c_buff, buf_sz), buf_sz, {});
        page_storage->write(std::move(batch));
    }

    {
        auto entry = page_storage->getEntry(page_id);
        ASSERT_EQ(entry.tag, tag);
        auto page = page_storage->read(page_id);
        for (size_t i = 0; i < buf_sz; ++i)
        {
            EXPECT_EQ(*(page.data.begin() + i), static_cast<char>(i % 0xff));
        }
    }

    auto done_full_gc = page_storage->gc();
    EXPECT_TRUE(done_full_gc);

    {
        auto entry = page_storage->getEntry(page_id);
        ASSERT_EQ(entry.tag, tag);
        auto page = page_storage->read(page_id);
        for (size_t i = 0; i < buf_sz; ++i)
        {
            EXPECT_EQ(*(page.data.begin() + i), static_cast<char>(i % 0xff));
        }
    }
}
CATCH

2. What did you expect to see? (Required)

the entry.tag remain unchanged after full gc

3. What did you see instead (Required)

new_entry.tag is not set after full gc

PageEntryV3 new_entry;
read(file_id, entry.offset, data_pos, entry.size, read_limiter, /*background*/ true);
// No need do crc again, crc won't be changed.
new_entry.checksum = entry.checksum;
// Need copy the field_offsets
new_entry.field_offsets = entry.field_offsets;
// Entry size won't be changed.
new_entry.size = entry.size;
new_entry.file_id = blobfile_id;
new_entry.offset = file_offset_beg + offset_in_data;

this will lead to more IO caused by RegionPersister::doPersist

if (page_reader)
{
auto entry = page_reader->getPageEntry(region_id);
if (entry.isValid() && entry.tag > applied_index)
return;

4. What is your TiFlash version? (Required)

master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-6.1 This bug affects the 6.1.x(LTS) versions. component/storage severity/major type/bug The issue is confirmed as a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants