Support flexiable num of L0 (blocking approach) #180
Conversation
| @@ -376,7 +376,7 @@ Status TableMgr::compactLevelItr(const CompactOptions& options, | |||
| // 1) number of files after split, and | |||
| // 2) min keys for each new file. | |||
| do { | |||
| if (!isCompactionAllowed()) { | |||
| if (!isCompactionAllowed() && !adjusting_num_l0) { | |||
There was a problem hiding this comment.
This is not acceptable. No matter what the reason is, compaction should be cancelled upon the close of DB instance.
| size_t level) | ||
| { | ||
| size_t level, | ||
| bool adjusting_num_l0) { |
There was a problem hiding this comment.
If passing adjusting_num_l0 is for blocking compaction cancellation, we should remove it.
| _log_err(myLog, "[Adjust numL0] not allowed in L0 only mode"); | ||
| throw Status(Status::INVALID_CONFIG); |
There was a problem hiding this comment.
It should not return error, but follow the previous behavior (accept the existing number).
| std::list<TableInfo*> tables; | ||
| s = mani->getL0Tables(ii, tables); | ||
| if (tables.size() != 1 || !s) { | ||
| _log_err(myLog, "[Adjust numL0] tables of hash %zu not found", ii); |
There was a problem hiding this comment.
Use [adjust numL0]. Using capital letter makes hassles when we search logs by keywords.
| // partitions. | ||
| std::list<TableInfo*> tables; | ||
| s = mani->getL0Tables(ii, tables); | ||
| if (tables.size() != 1 || !s) { |
There was a problem hiding this comment.
There can be more than 1 tables for the same hash, if previous L0 table was in the middle of compaction and the DB was closed.
| s = compactLevelItr(CompactOptions(), tables.back(), 0, true); | ||
| if (!s) { | ||
| _log_err(myLog, "[Adjust numL0] compaction error: %d", s); | ||
| throw s; | ||
| } | ||
| // The compacted table is remove from manifest in compactLevelItr, | ||
| // just release | ||
| for (TableInfo*& table: tables) { | ||
| table->done(); | ||
| } |
There was a problem hiding this comment.
This is not that simple problem. What if the process is force-closed (or crashed) in the middle so that a few tables are removed and the others are not, and then we reopen the DB? L0 is screwed up and how are you going to continue adjusting L0? Data loss or letting DB instance unavailable is not acceptable.
And this scenario should be thoroughly tested. Adjusting L0 is very risky and vulnerable.
No description provided.