Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to facebook 6.29 #277

Closed
39 tasks done
tabokie opened this issue May 9, 2022 · 8 comments
Closed
39 tasks done

Upgrade to facebook 6.29 #277

tabokie opened this issue May 9, 2022 · 8 comments
Assignees

Comments

@tabokie
Copy link
Member

tabokie commented May 9, 2022

Reason for 6.29:

  • Version above 6.27 has superior performance (especially read) according to official benchmarks
  • Not sure about facebook's backport policy, but 6.29 is getting more minor releases than predecessors

List of TiKV patches in chronological order:

Bullet-ed items are not cherry-picked, for either one of these reasons:

  • Build related
  • Already deprecated in 6.4.tikv (e.g. multi batch write)
  • Rendered unnecessary by new improvements from 6.29.fb (consistency check and cache key collision)
@tabokie tabokie changed the title Upgrade to facebook 6.28 Upgrade to facebook 6.29 May 9, 2022
@tabokie tabokie self-assigned this May 9, 2022
@hunterlxt
Copy link
Member

Are you going to pick our commits to 6.29? In what TiKV version are we going to start using 6.29?

@tabokie
Copy link
Member Author

tabokie commented May 10, 2022

@hunterlxt Soon I hope. Both multi-rocksdb and user timestamp require a newer version of RocksDB.

@tabokie
Copy link
Member Author

tabokie commented May 10, 2022

A list of facebook features that might interact with our old codebase, needs further scrutiny:

tabokie added a commit that referenced this issue May 11, 2022
Ref #277

When the iterator read keys in reverse order, each Prev() function cost O(log n) times. So I add prev pointer for every node in skiplist to improve the Prev() function.

Signed-off-by: Little-Wallace liuwei@pingcap.com

Implemented new virtual functions:
- `InsertWithHintConcurrently`
- `FindRandomEntry`

Signed-off-by: tabokie <xy.tao@outlook.com>
@hunterlxt
Copy link
Member

I found in v6.29 blobdb is introduced(rocksdb_options_set_enable_blob_files), maybe we can do some bench. cc @Connor1996

@Connor1996
Copy link
Member

blobdb has been already introduced before @hunterlxt

@hunterlxt
Copy link
Member

blobdb has been already introduced before @hunterlxt

But I don't find any FFI to rocksdb_options_set_enable_blob_files in rust-rocksdb

tabokie added a commit to tikv/tikv that referenced this issue Jul 12, 2022
Ref tikv/rocksdb#277

Added some configurations, they will be hidden from doc for now.
- rocksdb.xxcf.prepopulate-block-cache-mode = "disabled"
- rocksdb.xxcf.format-version = 2
- rocksdb.xxcf.checksum = "crc32c"
- WriteOptions::memtable_insert_hint_per_batch = false
- ReadOptions::auto_prefix_mode = false
- ReadOptions::adaptive_readahead = false

A few notes:
- `test_need_gc::test_without_properties` is removed, because in the new version of RocksDB, some portion of flushed data is replayed to memtable, and breaks the assumption of file layout. I haven't pinpointed the root cause, but I suppose this test case is not that important.
- `test_compact_files_in_range` is partially removed, because it raises error: `Invalid argument: Cannot compact file to up level, input file: /000032.sst level 6 > output level 3`.

Signed-off-by: tabokie <xy.tao@outlook.com>

Co-authored-by: 5kbpers <hustmh@gmail.com>
joccau pushed a commit to joccau/tikv that referenced this issue Jul 14, 2022
Ref tikv/rocksdb#277

Added some configurations, they will be hidden from doc for now.
- rocksdb.xxcf.prepopulate-block-cache-mode = "disabled"
- rocksdb.xxcf.format-version = 2
- rocksdb.xxcf.checksum = "crc32c"
- WriteOptions::memtable_insert_hint_per_batch = false
- ReadOptions::auto_prefix_mode = false
- ReadOptions::adaptive_readahead = false

A few notes:
- `test_need_gc::test_without_properties` is removed, because in the new version of RocksDB, some portion of flushed data is replayed to memtable, and breaks the assumption of file layout. I haven't pinpointed the root cause, but I suppose this test case is not that important.
- `test_compact_files_in_range` is partially removed, because it raises error: `Invalid argument: Cannot compact file to up level, input file: /000032.sst level 6 > output level 3`.

Signed-off-by: tabokie <xy.tao@outlook.com>

Co-authored-by: 5kbpers <hustmh@gmail.com>
@BusyJay
Copy link
Member

BusyJay commented Jul 24, 2022

Why #147 is not picked?

@tabokie
Copy link
Member Author

tabokie commented Jul 25, 2022

@BusyJay The feature is removed in case you forgot: tikv/titan#249

@tabokie tabokie closed this as completed Sep 29, 2022
v01dstar pushed a commit to v01dstar/rocksdb that referenced this issue Mar 5, 2024
Ref tikv#277

When the iterator read keys in reverse order, each Prev() function cost O(log n) times. So I add prev pointer for every node in skiplist to improve the Prev() function.

Signed-off-by: Little-Wallace liuwei@pingcap.com

Implemented new virtual functions:
- `InsertWithHintConcurrently`
- `FindRandomEntry`

Signed-off-by: tabokie <xy.tao@outlook.com>
Signed-off-by: v01dstar <yang.zhang@pingcap.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants