Add an option to dump wal seqno gaps #13014
Conversation
jowlyzhang
force-pushed
the
debug_624
branch
2 times, most recently
from
d6725a0
to
db28b91
Compare
jowlyzhang
force-pushed
the
debug_624
branch
from
72af2f3
to
63d48dd
Compare
There was a problem hiding this comment.
Thanks for the improvement @jowlyzhang ! LGTM overall, just some minor questions/comments
| std::string WALPicker::GetNextWAL() { | ||
| assert(Valid()); | ||
| std::string ret; | ||
| if (wal_file_iter_ != log_files_.end()) { |
There was a problem hiding this comment.
This check seems redundant, since we assert(Valid()); above
There was a problem hiding this comment.
I think this check is helpful if that assert line is not compiled in the build.
| DumpWalFile(options, dir_or_file, print_header, print_values, | ||
| only_print_seqno_gaps, is_write_committed, ucmps, exec_state, | ||
| &prev_batch_seqno, &prev_batch_count); |
There was a problem hiding this comment.
Question: what happens here if dir_or_file is an existing but empty directory?
There was a problem hiding this comment.
Good catch! The error message is not clear, I have added checks for it to have a legit log file name.
| row << sequence_number << ","; | ||
| row << batch_count << ","; |
There was a problem hiding this comment.
Question: so what the new option does is print the batch after the gap, right? Would it make sense to print some info about the start of the gap (prev_batch_seqno->value() + prev_batch_count->value()) as well?
There was a problem hiding this comment.
Yes, that's a good point. I have updated it print a line like this:
Prev batch sequence number: 163356, prev batch count: 1, 164172,1,112,78,PUT(6) : 0x00000000000224C9000000000000012B00000000000000EF18E44F7BD5721300
jowlyzhang
force-pushed
the
debug_624
branch
from
63d48dd
to
f62753d
Compare
|
@jowlyzhang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@jowlyzhang has updated the pull request. You must reimport the pull request before landing. |
|
@jowlyzhang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@jowlyzhang merged this pull request in 1238120. |
Add an option
--only_print_seqno_gapsfor wal dump to help with debugging. This option will check the continuity of sequence numbers in WAL logs, assumingseq_per_batchis false.--walfileoption now also takes a directory, and it will check all WAL logs in the directory in chronological order.When a gap is found, we can further check if it's related to operations like external file ingestion.
Test plan:
Manually tested