-
Notifications
You must be signed in to change notification settings - Fork 449
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
sstable: ingestion #2
Comments
justinj
pushed a commit
to justinj/pebble
that referenced
this issue
Feb 8, 2019
Fixes cockroachdb#36. This commit fixes two bugs in the record.LogWriter and adds tests for the LogWriter alongside the record.Writer. The first bug is that a record of length zero were skipped over and not written. I'm not sure if this behaviour is ever exercised in practice, but it is exercised by the tests for Writer so fixing it makes testing simpler. The second bug is the cause of cockroachdb#36: each chunk header is 7 bytes long, and no chunk can span block boundaries. When there is insufficient space left in a block to write an entire record, a "starting" chunk is written and we move onto the next block to write the rest of it. If there are exactly seven bytes available in the block, the original Writer would write the seven byte header despite not having any room to write any body bytes, while the LogWriter would omit them. This manifested as the Reader thinking that it was seeing a zeroed out block. This commit brings LogWriter's behaviour in line with Writer does, and what the Reader expects. I've also made a change suggested by @petermattis to error out in the case of a zero-block instead of attempting to recover. This required removing a test, and was also masking bug cockroachdb#2 in tests. The log should probably be truncated here and a log message generated stating that some data was likely lost (but we have recovered to a consistent point in time).
justinj
pushed a commit
to justinj/pebble
that referenced
this issue
Feb 8, 2019
Fixes cockroachdb#36. This commit fixes two bugs in the record.LogWriter and adds tests for the LogWriter alongside the record.Writer. The first bug is that a record of length zero were skipped over and not written. I'm not sure if this behaviour is ever exercised in practice, but it is exercised by the tests for Writer so fixing it makes testing simpler. The second bug is the cause of cockroachdb#36: each chunk header is 7 bytes long, and no chunk can span block boundaries. When there is insufficient space left in a block to write an entire record, a "starting" chunk is written and we move onto the next block to write the rest of it. If there are exactly seven bytes available in the block, the original Writer would write the seven byte header despite not having any room to write any body bytes, while the LogWriter would omit them. This manifested as the Reader thinking that it was seeing a zeroed out block. This commit brings LogWriter's behaviour in line with Writer does, and what the Reader expects. I've also made a change suggested by @petermattis to error out in the case of a zero-block instead of attempting to recover. This required removing a test, and was also masking bug cockroachdb#2 in tests. The log should probably be truncated here and a log message generated stating that some data was likely lost (but we have recovered to a consistent point in time).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Implement
DB.Ingest
for atomically ingesting a set of externally created sstables. The entries in the sstable are all given the same sequence number using the global-seqnum sstable property. That means the ingested sstables must not self overlap. Sstables should be ingested into the lowest level of the LSM tree possible, though that might mean flushing the memtable and ingesting them into L0.The text was updated successfully, but these errors were encountered: