TestLogCopySetsCloseTimestampInFooter unit test failing with high frequency #24518
Closed
1 task done
Assignees
Labels
2.20 Backport Required
2024.1 Backport Required
2024.2 Backport Required
area/docdb
YugabyteDB core features
kind/bug
This issue is a bug
priority/medium
Medium priority issue
QA
QA filed bugs
Comments
agsh-yb
added
area/documentation
rthallamko3
added
area/docdb
yugabyte-ci
added
kind/enhancement
rthallamko3
added
kind/bug
basavaraj29
added a commit
that referenced
this issue
Nov 5, 2024
…mpInFooter
Summary:
Test `TabletSplitITest.TestLogCopySetsCloseTimestampInFooter` was put up to assert that the footer timestamp was being rightly set for log segments when duplicating the segment for a split child tablet.
The test was trying to make the following assertion
```
for (const auto& entry : entries_copy_result.entries) {
if (entry->has_replicate()) {
has_replicated_entries = true;
break;
}
}
ASSERT_EQ(has_replicated_entries, segment->footer().has_close_timestamp_micros())
<< "T " << peer->tablet_id() << " P " << peer->permanent_uuid()
<< ": Expected valid close timestamp for segment with replicated entries.";
}
```
where we assert that the segment would have a valid footer timestamp only when the `segment->HasFooter()` and the segment has non-zero replicate entries. But from the code, we can see that there exists cases where a segment has a valid footer and has 0 replicate entries
```
if (!footer_builder_.has_min_replicate_index()) {
VLOG_WITH_PREFIX(1) << "Writing a segment without any REPLICATE message. Segment: "
<< active_segment_->path();
}
```
Additionally, detective failure logs confirm this to be the case
```
../../src/yb/integration-tests/tablet-split-itest.cc:966: Failure
Expected equality of these values:
has_replicated_entries
Which is: false
segment->footer().has_close_timestamp_micros()
Which is: true
```
The diff fixes the test issue by asserting that the log segment has a valid footer close timestamp when `segment->HasFooter()` is true. This assertion should hold since the footer should always have a valid close timestamp.
Jira: DB-13553
Test Plan:
Jenkins: compile-only
./yb_build.sh release --cxx-test='TEST_F(TabletSplitITest, TestLogCopySetsCloseTimestampInFooter) {' -n 50 --tp 1
Note: The modified test rightly fails when source code changes fixing #23335 are reverted, which is the required behavior.
Reviewers: timur, rthallam
Reviewed By: timur
Subscribers: ybase
Differential Revision: https://phorge.dev.yugabyte.com/D39615
basavaraj29
added a commit
that referenced
this issue
Nov 5, 2024
…ySetsCloseTimestampInFooter Summary: Original commit: 4785122 / D39615 Test `TabletSplitITest.TestLogCopySetsCloseTimestampInFooter` was put up to assert that the footer timestamp was being rightly set for log segments when duplicating the segment for a split child tablet. The test was trying to make the following assertion ``` for (const auto& entry : entries_copy_result.entries) { if (entry->has_replicate()) { has_replicated_entries = true; break; } } ASSERT_EQ(has_replicated_entries, segment->footer().has_close_timestamp_micros()) << "T " << peer->tablet_id() << " P " << peer->permanent_uuid() << ": Expected valid close timestamp for segment with replicated entries."; } ``` where we assert that the segment would have a valid footer timestamp only when the `segment->HasFooter()` and the segment has non-zero replicate entries. But from the code, we can see that there exists cases where a segment has a valid footer and has 0 replicate entries ``` if (!footer_builder_.has_min_replicate_index()) { VLOG_WITH_PREFIX(1) << "Writing a segment without any REPLICATE message. Segment: " << active_segment_->path(); } ``` Additionally, detective failure logs confirm this to be the case ``` ../../src/yb/integration-tests/tablet-split-itest.cc:966: Failure Expected equality of these values: has_replicated_entries Which is: false segment->footer().has_close_timestamp_micros() Which is: true ``` The diff fixes the test issue by asserting that the log segment has a valid footer close timestamp when `segment->HasFooter()` is true. This assertion should hold since the footer should always have a valid close timestamp. Jira: DB-13553 Test Plan: Jenkins: compile-only ./yb_build.sh release --cxx-test='TEST_F(TabletSplitITest, TestLogCopySetsCloseTimestampInFooter) {' -n 50 --tp 1 Note: The modified test rightly fails when source code changes fixing #23335 are reverted, which is the required behavior. Reviewers: timur, rthallam Reviewed By: rthallam Subscribers: ybase Differential Revision: https://phorge.dev.yugabyte.com/D39731
basavaraj29
added a commit
that referenced
this issue
Nov 5, 2024
…ySetsCloseTimestampInFooter Summary: Original commit: 4785122 / D39615 Test `TabletSplitITest.TestLogCopySetsCloseTimestampInFooter` was put up to assert that the footer timestamp was being rightly set for log segments when duplicating the segment for a split child tablet. The test was trying to make the following assertion ``` for (const auto& entry : entries_copy_result.entries) { if (entry->has_replicate()) { has_replicated_entries = true; break; } } ASSERT_EQ(has_replicated_entries, segment->footer().has_close_timestamp_micros()) << "T " << peer->tablet_id() << " P " << peer->permanent_uuid() << ": Expected valid close timestamp for segment with replicated entries."; } ``` where we assert that the segment would have a valid footer timestamp only when the `segment->HasFooter()` and the segment has non-zero replicate entries. But from the code, we can see that there exists cases where a segment has a valid footer and has 0 replicate entries ``` if (!footer_builder_.has_min_replicate_index()) { VLOG_WITH_PREFIX(1) << "Writing a segment without any REPLICATE message. Segment: " << active_segment_->path(); } ``` Additionally, detective failure logs confirm this to be the case ``` ../../src/yb/integration-tests/tablet-split-itest.cc:966: Failure Expected equality of these values: has_replicated_entries Which is: false segment->footer().has_close_timestamp_micros() Which is: true ``` The diff fixes the test issue by asserting that the log segment has a valid footer close timestamp when `segment->HasFooter()` is true. This assertion should hold since the footer should always have a valid close timestamp. Jira: DB-13553 Test Plan: Jenkins: compile-only ./yb_build.sh release --cxx-test='TEST_F(TabletSplitITest, TestLogCopySetsCloseTimestampInFooter) {' -n 50 --tp 1 Note: The modified test rightly fails when source code changes fixing #23335 are reverted, which is the required behavior. Reviewers: timur, rthallam Reviewed By: rthallam Subscribers: ybase Differential Revision: https://phorge.dev.yugabyte.com/D39732
basavaraj29
added a commit
that referenced
this issue
Nov 5, 2024
…etsCloseTimestampInFooter Summary: Original commit: 4785122 / D39615 Test `TabletSplitITest.TestLogCopySetsCloseTimestampInFooter` was put up to assert that the footer timestamp was being rightly set for log segments when duplicating the segment for a split child tablet. The test was trying to make the following assertion ``` for (const auto& entry : entries_copy_result.entries) { if (entry->has_replicate()) { has_replicated_entries = true; break; } } ASSERT_EQ(has_replicated_entries, segment->footer().has_close_timestamp_micros()) << "T " << peer->tablet_id() << " P " << peer->permanent_uuid() << ": Expected valid close timestamp for segment with replicated entries."; } ``` where we assert that the segment would have a valid footer timestamp only when the `segment->HasFooter()` and the segment has non-zero replicate entries. But from the code, we can see that there exists cases where a segment has a valid footer and has 0 replicate entries ``` if (!footer_builder_.has_min_replicate_index()) { VLOG_WITH_PREFIX(1) << "Writing a segment without any REPLICATE message. Segment: " << active_segment_->path(); } ``` Additionally, detective failure logs confirm this to be the case ``` ../../src/yb/integration-tests/tablet-split-itest.cc:966: Failure Expected equality of these values: has_replicated_entries Which is: false segment->footer().has_close_timestamp_micros() Which is: true ``` The diff fixes the test issue by asserting that the log segment has a valid footer close timestamp when `segment->HasFooter()` is true. This assertion should hold since the footer should always have a valid close timestamp. Jira: DB-13553 Test Plan: Jenkins: compile-only ./yb_build.sh release --cxx-test='TEST_F(TabletSplitITest, TestLogCopySetsCloseTimestampInFooter) {' -n 50 --tp 1 Note: The modified test rightly fails when source code changes fixing #23335 are reverted, which is the required behavior. Reviewers: timur, rthallam Reviewed By: rthallam Subscribers: ybase Differential Revision: https://phorge.dev.yugabyte.com/D39733
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Jira Link: DB-13553
Description
TabletSplitITest.TestLogCopySetsCloseTimestampInFooter starts failing recently on alma8-clang17-release-aarch64 Analyze_trend with flakiness
Warning: Please confirm that this issue does not contain any sensitive information
The text was updated successfully, but these errors were encountered: