update blob_db_test #2375
update blob_db_test #2375
Conversation
|
@yiwu-arbug updated the pull request - view changes |
|
@yiwu-arbug has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
|
@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. |
utilities/blob_db/blob_db_test.cc
Outdated
| if (!len) continue; | ||
| void PutRandomWithTTL(const std::string &key, int32_t ttl, Random *rnd, | ||
| std::map<std::string, std::string> *data = nullptr) { | ||
| int len = rnd->Next() % 16384 + 1; |
There was a problem hiding this comment.
16384 is 2^16. (2 >> 16) would be much more readable here imo instead of 16384; the reason being, the first time i saw this line, I was left wondering what this magic number is for a brief moment ... and other people might have the same thought.
| std::string value; | ||
|
|
||
| ColumnFamilyHandle *dcfh = blobdb_->DefaultColumnFamily(); | ||
| Reopen(bdboptions); | ||
|
|
||
| Random rnd(301); |
There was a problem hiding this comment.
Curious, why 301? Is it just a random magic number? May be just define it.
There was a problem hiding this comment.
Just an arbitrary seed. I'm hesitate to define a constant for it, because it is arbitrary and different test can use different seed.
|
@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. |
utilities/blob_db/blob_db_test.cc
Outdated
| @@ -38,6 +38,8 @@ void gen_random(char *s, const int len) { | |||
|
|
|||
| class BlobDBTest : public testing::Test { | |||
| public: | |||
| constexpr int kMaxBlobSize = 1 << 13; | |||
There was a problem hiding this comment.
Not 1 << 14 ?
16384 == 1<<14.
|
@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: Re-enable blob_db_test with some update: * Commented out delay at the end of GC tests. Will update the logic later with sync point to properly trigger GC. * Added some helper functions. Also update make files to include blob_dump tool. Closes #2375 Differential Revision: D5133793 Pulled By: yiwu-arbug fbshipit-source-id: 95470b26d0c1f9592ba4b7637e027fdd263f425c
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:
Re-enable blob_db_test with some update:
Also update make files to include blob_dump tool.
Test Plan:
make all check