Cherry-pick TiKV related changes to 8.10.fb

[v01dstar]

6.29 (last TiKV base) diff: facebook/rocksdb@6.29.fb...tikv:rocksdb:6.29.tikv

• Apply write-amplification-based rate limiter fe76269
  • 3dc1b55
  • 458bbd7
  • ccf5215
  • f9aacb3
  • 4751586
  • 8d6414e
  • db59a3f
  • c0953b3
  • 938c016
  • Comments:
    • Env::IOPriority has more options now, LOW, MID, HIGH, USER instead of just LOW and HIGH, write amplification rate limiter's tuning logic may need to adjust to this change.
• Apply PerfFlag patch b27b564
  • 2f1efc5
  • 7ee5329
  • Comments:
    • We need to resolve compatibility issue every time when RocksDB adds new metrics. And all dependent projects, like Titan and rust-rocksdb have to change accordingly.
• Compaction filter optimization 2a93687 <-re-evaluate unsafe filter v4
  • 8e20349
  • 9554ad2
  • 23c8635 <- removed (this is a bug)
  • 3a238bf <- no longer needed, see avoid deadlock for GC in compaction filter tikv#9694
  • Manual apply related changes in bb515db
  • Comments:
    • We also need to resolve compatibility issue every time for this when RocksDB changes filter API.
• Doubly skiplist for reverse scan acc80ce
  • 5d01038
• Add WAL write duration metric de0fc30
  • e3c8f48
• Add Iterator and Append method for WriteBatch 6ffdf20
  • 340b810
• TiKV IO rate limiter a8d22fc
  • 7749860
  • 52b4a97 <- retiring because of Add subcompaction event API facebook/rocksdb#9311
  • 4ec4a1f
  • 8bd42ca <- removed base_background_compactions related code due to Remove unused API base_background_compactions facebook/rocksdb#9462
  • 28f7636
  • Remove ROCKSDB_LITE macro use
• Manifest dump tool optimization
  • e1797a0
  • 615c1dc
• Implement pipelined commit / multi-batch write
  • 910417b
  • 19db40b
  • 2d03d53 <- removed, but we need to verify this does not cause upgrade issue again tikv restart failed（upgrade new image and CrashLoopBackOff） tikv#13007
  • f4cba2f
  • 9bb7147
  • Manually applied some changes from 98a80e9
  • Comments:
    • This is not easy to maintain, multi-batch write's implementation is based on rocksdb::BatchWrite() (with some custom code). Whenever rocksdb::BatchWrite changes, multi-batch write's implementation should change accordingly, this requires human intervention.
• Per-file encryption key management
  • b9c2064
  • 6386992
  • 63399df
  • 63586f2
  • 7ee32c0
  • bbd27cf
  • 1868d12
  • 4cebfc1
  • 9464766
  • Remove ROCKSDB_LITE macro
• Optimize SST partitioner to avoid huge compaction
  • e2f6ec7
• Titan
  • Make statistics extensible 6d88b39
    • 7c6dcaa <- pitfall, do not try to move statistics impl to stats.cc. https://stackoverflow.com/questions/1111440/undefined-reference-error-for-template-method
    • Manual apply changes in bb515db
  • Manual apply related changes (blob index) in bb515db
  • dcf2f8d
• Raftstore v2?
  • 32f8f2b
  • faad483
  • 8899a36 <- remove wal
  • 638c217
  • Expose seqno / Add post write callback
    • 3cd757c <- over-written by 08aa503
    • 08aa503
    • fdcd14d
  • 9ea79ab
  • acc624f
  • cd9aa99
  • 8a9c10e
  • 14f36f8
  • 5b9cef9 <-re-evaluate
  • de47e8e
  • 6121b2d
  • 0813e37
  • fe76937 just the CheckInRange API

Already exist in upstream (fb):

• d33a7c7
• 2ac42c6
• 310c818
• 61809fe
• 1726df0
• 9c86bf5
• ee97291
• 8b1ab20
• 8c7ff07
• 267baed
• d453c8e
• e33a233
• 22304c4
• 1022df7
• 98a80e9
• 6a7472a
• ce0ea61
• 4d12477
• 45509f0
• 20225a4
• 3dba9fa

No longer needed:

• bc1f255
• dc9353f <- fixed differently in upstream OnFlushCompleted is called before flush completed facebook/rocksdb#5892
• 545d0b2 <- fixed by Sort L0 files by newly introduced epoch_num facebook/rocksdb#10922
• bb515db <- Separated. i.e. monitoring related changes are merged with "Make statistics extensible"
• 2cbb069 <- reverted by 53eae82
• 53eae82 <- reverting 2cbb069

Need triage:

• 0559eac <- rocksdb cloud

To be verified:

• 40551e2 <- we can evaluate RocksDB's new option for solving this problem introduced in Delay bottommost level single file compactions facebook/rocksdb#11701

Complications:

• WriteBufferManager has changed a lot
• SST file epoch number was introduced, instance merge needs to accommodate that change.
• Write stall logic behavior change introduced in multi-instance support project made some tests to fail.
• RocksDB now uses C++17 standard

[hbisheng]

It looks like tikv ran into a segfault on this line. I'm still trying to understand why but I guess it's related to the addition of new IO priorities.

Here's the evidence:

1. segfault information:

[Mon Jun 24 10:59:14 2024] apply-1[2562783]: segfault at 7f68952f40a0 ip 000055bd6eacc09a sp 00007f6c1d621f90 error 6 in tikv-server[55bd69e00000+6052000]

2. The static address of the segfault code can be calculated as ip (000055bd6eacc09a) - base_addr_of_tikv (55bd69e00000) = 0x4ccc09a.
3. With the help of gdb, we can locate the code of that address.

$ gdb tikv-server
(gdb) info line *0x4ccc09a
Line 172 of "/workspace/.cargo/git/checkouts/rust-rocksdb-9e01d192e8b6561d/af14652/librocksdb_sys/rocksdb/utilities/rate_limiters/write_amp_based_rate_limiter.cc"                                                                                               
   starts at address 0x4ccc093 <rocksdb::WriteAmpBasedRateLimiter::Request(long, rocksdb::Env::IOPriority, rocksdb::Statistics*)+147>                                                                                                                            
   and ends at 0x4ccc0a2 <rocksdb::WriteAmpBasedRateLimiter::Request(long, rocksdb::Env::IOPriority, rocksdb::Statistics*)+162>.

[v01dstar]

Rocksdb introduced more types of priorities (User, Mid etc) since 8.x, while WriteAmpRateLimiter only considered 3 of them. I thought it could work, but maybe this can cause some problems.

[hbisheng]

One thing that seems to be interesting: the segfault issue seemed to only happen on Linux machines; I wasn't able to reproduce it on Mac. So it could be arch/compiler related.

Also, I managed to get a coredump of the segfault on Linux.

Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x00005595b5961efa in rocksdb::WriteAmpBasedRateLimiter::Request (this=0x7ffa1e214000, bytes=46541, pri=<optimized out>, stats=0x0)
    at /root/tabokie/packages/cargo/.cargo/git/checkouts/rust-rocksdb-da3ff04b5d606849/ca1f1dd/librocksdb_sys/rocksdb/utilities/rate_limiters/write_amp_based_rate_limiter.cc:172
172	  ++total_requests_[pri];

It shows that total_requests_ was initialized with a length of 4 as expected. But the pri was optimized out.

(gdb) print total_requests_
$1 = {1407, 16, 274, 0}
(gdb) print pri
$2 = <optimized out>

Given the definition of IOPriority, the only way for it to cause a segfault is when pri equals IO_TOTAL, but I don't think that's how we expect pri to be used...

  enum IOPriority {
    IO_LOW = 0,
    IO_MID = 1,
    IO_HIGH = 2,
    IO_USER = 3,
    IO_TOTAL = 4
  };

Still investigating...

[hbisheng]

Found the problem! Turns out that one constructor for Writer did not initialize rate_limiter_priority. With this one-line fix, the segfault problem went away.

We might want to check how the bug was introduced and whether there could be other similar problems.
Update: The upstream 8.10.fb branch does not have this problem (db/write_thread.h), so it's likely an oversight when we cherry-picked the commits.

[Connor1996]

👍