[PCP-21] Titan GC doesn’t affect online write

[tabokie]

Green Blob GC by merge operator

tikv/tikv#5739

• New value type: kMergeBlobIndex
• New value content: LSN + BlobIndex

Merge Semantics: keep freshest version

A. key + merged index: source-index must be older than key, apply key

B. index + merged index: if index is stale (GCed), apply merged-index or simply delete key; else apply index

implementation:

• if any merge-index is sourced from base-index, it's certain that base-index is stale.
• source-file-number info can be stored in merge-blob-index

C. merged index + merged index (merge against delete): deletes key (use deletion marker instead)

D. merged index + merged index (partial merge): by LSN version of original put.

implementation:

• embed LSN in merge operand, during blob building append LSN to blob key, during GC pass this LSN to merge operand (also to new blob file).
• embed LSN in blob key, when conflict read blob for latest index. (partial merge must read blob)

GC workflow:

Three approaches to merge new index into rocksdb, requires further benchmarking:

• plain merge
• ingestion
• fast ingestion
  • write SST of newly created blob index as Merge op
  • ingest external SST in a weak consistent way (do not pause write and flush on overlap, only guarantee ingested file is latest in persistent level)

Rocksdb Change:

tikv/rocksdb#147

• support kTypeBlobIndex in merge operation (to support user-specified merge on top of gc-merge, extend merge value to include operand type)
• add value type input/output to merge interface
• merge operator deletes key
• skip_memtable_check flag in IngestOption

[tabokie]

A bug is spotted in current work: merge's correctness depends on retrieving up-to-date logical file state, meanwhile the file state transit depends on all new indexes being inserted so we can use LatestSequenceNumber as obsolete sequence. The problem is tricky to solve given titan inclines to atomically change file state and persist it to log.
New idea is to avoid file state inspection (which also save us bunch of locking). Instead, embed old file number to merge operands, so that base index is ignored if sharing the same file number with merges (announcing itself the source index producing this merge operand).
New value content is now LSN + old file number + BlobIndex

[tabokie]

ready for review now @yiwu-arbug @Connor1996. benchmark coming soon.

[yiwu-arbug]

@tabokie Awesome! Let's see how benchmark is doing.

[tabokie]

@yiwu-arbug @Connor1996 Here are some test results, Write performance brought by new gc is impressive, while gc-with-merge is more feasible with read regression below 10 percent.

dbbench (./titandb_bench --key_size="16" --value_size="256" --cache_size="4294967296" --titan_min_blob_size="0" --num="10000000" --numdistinct="10000" --benchmarks="fillseq,overwrite,readrandom" --threads="6" --titan_max_background_gc="6"):
gc_rewrite_mode = kDefault
fillseq: 99.3 MB/s; overwrite: 23.2 MB/s; readrandom: 130.5 MB/s
gc_rewrite_mode = kMerge
fillseq: 102.5 MB/s; overwrite: 59.3 MB/s; readrandom: 122.8 MB/s
gc_rewrite_mode = kFastIngest
fillseq: 101.1 MB/s; overwrite: 61.8 MB/s; readrandom: 82.4 MB/s

go-ycsb (max-background-gc=4, threads=256, readcount=1kw, operationcount=2kw)
gc_rewrite_mode = kDefault
update ops 5w -> 4.53w, read ops 12w
gc_rewrite_mode = kMerge
update ops 5w -> 4.58w, read ops 11w
gc_rewrite_mode = kFastIngest
update ops 5w -> 4.7w, read ops 7w

[codecov-io]

Codecov Report

Merging #121 into master will decrease coverage by 0.56%.
The diff coverage is 85.38%.

@@            Coverage Diff             @@
##           master     #121      +/-   ##
==========================================
- Coverage   86.02%   85.46%   -0.57%     
==========================================
  Files          45       45              
  Lines        3528     3419     -109     
==========================================
- Hits         3035     2922     -113     
- Misses        493      497       +4

[yiwu-arbug]

@tabokie As we are approaching the end of PCP, we need to merge this PR this week. To speed it up, let's do the following:

1. remove all ingest file related logic to make review simpler.
2. send PR for related rocksdb changes to tikv/rocksdb repo.
3. default to turn off the feature. we can add option to option.h later.
4. adding sequence to blob record is a breaking change. can you remind us why it is needed? I'm incline to remove it for now, even if it will introduce correctness issue.
5. can you remind us on the merge semantics? I find the following in the old docs but it is still unclear to me:

This is great work overall! We can keep working on remaining work after PCP. We definitely want it to included in tikv 4.0 release.

[tabokie]

@yiwu-arbug Some Q&A first:

1. Embedding LSN in blob record is a critical design choice. In this snippet, merge operator avoids losing blob index by only keeping the latest one in multiple merge operands. Here, latest refer to timestamp of the original Put, which is a meaningful concept only on titan end, and therefore required to be stored in blob record payload. In detail, this blob timestamp is flowing in this path: MemKey ->[table-builder/GC-by-put]-> BlobRecord ->[GC-by-merge/merge]->MergeOperand. Notice the GC-by-put mutation is causing mis-alignment between the two timestamp.
2. On a second thought, merge semantics is better explained by describing the anomalies it tries to avoid ([X] refers to put, (X) refers to merge, full content is [X blob-ts source-fno value-fno], time is flowing rightwards):
   *. basic rule, keep the base value: [?] ... [X] (Y) (Z) => [X]
   this is mostly true when Y/Z stem from older ?, guaranteed by rocksdb visibility constraint.
   a. base + delta: [Y] ... [X] (Y) (X') => [X']
   simple equal case where X' stem from exactly X. We identify this case by checking the source-file-number stored in merge operands, in detail: [X - - 0] (Y) (X' - 0 1).
   b. merge reorder: [A] [B] (B') (A') => [B']
   visibility order of merge != that of value. We need to preserve original order in reordered merge operands (explained before). in detail: [A 0 - -] [B 1 - -] (B' 1 - -) (A' 0 - -)
   c. deletion: [delete] (X) (Y) => [delete marker]
   introducing deletion marker is a workaround against a restriction that rocksdb merge can't produce empty result.

[tabokie]

Almost good now, PTAL @yiwu-arbug @Connor1996

[yiwu-arbug]

I may be missing something, but I still think merge operator don't need to know LSN of values. Just like the write callback for gc-with-put doesn't need to know LSN of keys. I think we can simply use the following logic in merge op (pseudo-code):

blob_index = existing_value;
for (merge_op : in_merge_op_list) {
    if (merge_op.source_file == blob_index.file && merge_op.source_offset == blob_index.offset) {
        blob_index.file = merge_op.file;
        blob_index.offset = merge_op.offset
    }
}
merge_output = blob_index;

[Connor1996]

rest LGTM

[Connor1996]

LGTM, let's merge rocksdb first

[yiwu-arbug]

LGTM. Leave to @Connor1996 for merge.

[zhangjinpeng87]

@Connor1996 Can we merge it now?

[Connor1996]

LGTM