Fixing blob db sequence number handling #2385
Conversation
|
@yiwu-arbug has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
|
@siying Mind take a look at the change to the core write path? Thanks! |
| @@ -919,7 +920,7 @@ Status BlobDBImpl::Write(const WriteOptions& opts, WriteBatch* updates) { | |||
| Writer::ConstructBlobHeader(&headerbuf, key, value, expiration, -1); | |||
|
|
|||
| if (previous_put) { | |||
| impl->AppendSN(last_file, -1); | |||
| impl->AppendSN(last_file, 0 /*sequence number*/); | |||
There was a problem hiding this comment.
@anirbanr-fb I'm using 0 instead of -1 for dummy sequence number here, since SequenceNumber is an unsigned integer. Will GC be fine with it?
db/db_test_util.cc
Outdated
| break; | ||
| } | ||
| case kConcurrentMemTableWrite: { | ||
| options.allow_concurrent_memtable_write = true; |
There was a problem hiding this comment.
If I remember correctly, this is true by default.
db/db_test_util.cc
Outdated
| } | ||
| case kPipelinedWriteAndConcurrentMemTableWrite: { | ||
| options.enable_pipelined_write = true; | ||
| options.allow_concurrent_memtable_write = true; |
There was a problem hiding this comment.
Since allow_concurrent_memtable_write is true by default, I think this is the same as the previous case.
|
@yiwu-arbug updated the pull request - view changes - changes since last import |
|
@yiwu-arbug updated the pull request - view changes - changes since last import |
|
@yiwu-arbug has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
Summary: Blob db rely on base db returning sequence number through write batch after DB::Write(). However after recent changes to the write path, DB::Writ()e no longer return sequence number in some cases. Fixing it by have WriteBatchInternal::InsertInto() always encode sequence number into write batch. Stacking on #2375. Closes #2385 Differential Revision: D5148358 Pulled By: yiwu-arbug fbshipit-source-id: 8bda0aa07b9334ed03ed381548b39d167dc20c33
Summary:
Blob db rely on base db returning sequence number through write batch after DB::Write(). However after recent changes to the write path, DB::Writ()e no longer return sequence number in some cases. Fixing it by have WriteBatchInternal::InsertInto() always encode sequence number into write batch.
Stacking on #2375.
Test Plan:
See the two new tests. DBWriteTest::ReturnSequenceNumber tests DB::Write() return sequence number through write batch. BlobDBTest::SequenceNumber tests the sequence number encoded in blob file is correct.