feat(storage): change the prefix_hint to dist_key_hint for bloom_filter

[wcy-fdu]

I hereby agree to the terms of the Singularity Data, Inc. Contributor License Agreement.

Background

After the watermark design:

1. we currently only use the bloom filter when the distribution key is a prefix of pk and the state store needs a prefix_hint which supposed to be the distribution key
2. the time column might not be a part of the distribution_key because it might be a hotspot
3. we need time col as pk prefix to do fast state cleaning

2 and 3 means the distribution key will not be the prefix of pkpkwhich is conflict with 1.

Proposal

Bloom filter always uses distribution key, so distribution key do not need to be the prefix of pk.

After we break the assumption and distribution key will may not be the prefix of pk, there will be the following changes:

1. In StateTable/StorageTable, previous we check bloom filter only when key_indices==dist_key_indices, now we only need to judge whether distribution_key is the subset of pk.
2. Previously, as distribution_key is prefix, the bloom filter key is also the prefix of FullKey, which means the bloom filter key contains TablePrefix and VnodePrefix(For convenience slice. In new design, bloom filter key is no need to contains TablePrefix and VnodePrefix. So in a word, bloom filter key is distribution_key.
   Note that we may implement per table bloom filter after storage: improve SST builder with FullKey struct #6391
3. Based on 2,

• we need to remove table_id and vnode in prefix_hint, which is named dist_key_hint in this PR. vnode is connected in relational table layer, table_id is connected in hummock.
• we can not do such assertions:

  risingwave/src/storage/src/hummock/state_store.rs

  Lines 140 to 182 in ad03b2e

  	let next_key = next_key(prefix_hint);
  	// learn more detail about start_bound with storage_table.rs.
  	match key_range.start_bound() {
  	// it guarantees that the start bound must be included (some different case)
  	// 1. Include(pk + col_bound) => prefix_hint <= start_bound <
  	// next_key(prefix_hint)
  	//
  	// for case2, frontend need to reject this, avoid excluded start_bound and
  	// transform it to included(next_key), without this case we can just guarantee
  	// that start_bound < next_key
  	//
  	// 2. Include(next_key(pk +
  	// col_bound)) => prefix_hint <= start_bound <= next_key(prefix_hint)
  	//
  	// 3. Include(pk) => prefix_hint <= start_bound < next_key(prefix_hint)
  	Included(range_start) | Excluded(range_start) => {
  	assert!(range_start.as_slice() >= prefix_hint.as_slice());
  	assert!(range_start.as_slice() < next_key.as_slice() || next_key.is_empty());
  	}
  	_ => unreachable!(),
  	}
  	match key_range.end_bound() {
  	Included(range_end) => {
  	assert!(range_end.as_slice() >= prefix_hint.as_slice());
  	assert!(range_end.as_slice() < next_key.as_slice() || next_key.is_empty());
  	}
  	// 1. Excluded(end_bound_of_prefix(pk + col)) => prefix_hint < end_bound <=
  	// next_key(prefix_hint)
  	//
  	// 2. Excluded(pk + bound) => prefix_hint < end_bound <=
  	// next_key(prefix_hint)
  	Excluded(range_end) => {
  	assert!(range_end.as_slice() > prefix_hint.as_slice());
  	assert!(range_end.as_slice() <= next_key.as_slice() || next_key.is_empty());
  	}
  	std::ops::Bound::Unbounded => {
  	assert!(next_key.is_empty());
  	}

Checklist

• I have written necessary rustdoc comments
• I have added necessary unit tests and integration tests
• All checks passed in ./risedev check (or alias, ./risedev c)

Documentation

If your pull request contains user-facing changes, please specify the types of the changes, and create a release note. Otherwise, please feel free to remove this section.

Types of user-facing changes

Please keep the types that apply to your changes, and remove those that do not apply.

• Installation and deployment
• Connector (sources & sinks)
• SQL commands, functions, and operators
• RisingWave cluster configuration changes
• Other (please specify in the release note below)

Release note

Please create a release note for your changes. In the release note, focus on the impact on users, and mention the environment or conditions where the impact may occur.

Refer to a related PR or issue link (optional)

close #6288

[wcy-fdu]

Just found it a little hard to verify whether these changes are correct, so I opened this PR, hoping to detect it through CI. Sorry forgot to draft it...
I will check the comments later and invite review after all CI tests have passed😊.

[codecov]

Codecov Report

Merging #6575 (e524946) into main (0439b74) will increase coverage by 0.01%.
The diff coverage is 79.10%.

@@            Coverage Diff             @@
##             main    #6575      +/-   ##
==========================================
+ Coverage   73.22%   73.23%   +0.01%     
==========================================
  Files        1024     1024              
  Lines      163823   163891      +68     
==========================================
+ Hits       119960   120033      +73     
+ Misses      43863    43858       -5     

Flag	Coverage Δ
rust	73.23% <79.10%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/ctl/src/cmd_impl/hummock/list_kv.rs	0.00% <0.00%> (ø)
src/storage/hummock_test/src/failpoint_tests.rs	0.00% <0.00%> (ø)
src/storage/hummock_test/src/snapshot_tests.rs	65.10% <ø> (ø)
src/storage/src/hummock/state_store.rs	76.92% <ø> (+0.20%)	⬆️
src/storage/src/store.rs	69.64% <ø> (ø)
src/storage/src/table/batch_table/storage_table.rs	82.18% <31.81%> (-2.32%)	⬇️
src/stream/src/common/table/state_table.rs	82.57% <48.64%> (-0.60%)	⬇️
src/common/src/catalog/internal_table.rs	83.33% <72.41%> (-16.67%)	⬇️
src/storage/hummock_test/src/state_store_tests.rs	80.16% <76.66%> (ø)
...rc/storage/hummock_sdk/src/filter_key_extractor.rs	90.48% <91.42%> (-0.49%)	⬇️
... and 19 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[wcy-fdu]

Hi guys, this PR can be reviewed now, welcome to leave your comments. cc @hzxa21 @Li0k @st1page

[hzxa21]

twox-hash and xxhash-rust are both used to calculate xxhash. Let's pick one and only use one lib for xxhash.

[wcy-fdu]

Let's replace twox-hash with xxhash-rust it in next PR.

[hzxa21]

get_min_dist_key_start_index_if_continuous?

[wcy-fdu]

start_index is always min

[hzxa21]

Rest LGTM. Good job! Thanks for the PR!