refactor(common): move some over window utils to common

[stdrc]

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

We had some handy utils and helper types in over_window module before, including KeyWithSentinel (renamed to Sentinelled), EstimatedBTreeMap and DeltaBTreeMap. This PR moves:

• Sentinelled to common/types;
• EstimatedBTreeMap to common/estimate_size;
• DeltaBTreeMap to utils/delta_btree_map.

Checklist

• I have written necessary rustdoc comments
• I have added necessary unit tests and integration tests
• I have added test labels as necessary. See details.
• I have added fuzzing tests or opened an issue to track them. (Optional, recommended for new SQL features Sqlsmith: Sql feature generation #7934).
• My PR contains breaking changes. (If it deprecates some features, please create a tracking issue to remove them in the future).
• All checks passed in ./risedev check (or alias, ./risedev c)
• My PR changes performance-critical code. (Please run macro/micro-benchmarks and show the results.)

• My PR contains critical fixes that are necessary to be merged into the latest release. (Please check out the details)

Documentation

• My PR needs documentation updates. (Please use the Release note section below to summarize the impact on users)

Release note

If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.

[xxchan]

Where can they be reused?

[stdrc]

Where can they be reused?

Idk, but

• for KeyWithSentinel, I found it needed by another over_window submodule being implementing which indicates it's generality (at least some extent of);
• for EstimatedBTreeMap, it's very common and has nothing related to over window, it's reasonable to move it to common following the convention of EstimatedHashMap and EstimatedVecDeque;
• for DeltaBTreeMap, we have a similar but with different purpose struct in meta, BTreeMapTransaction, which focuses on insert/delete/get operations while DeltaBTreeMap focuses on get/iter operations. This indicates that this kind of helper types may be needed by other modules, so moving it to common can prevent potential re-invention.

[xxchan]

It would be better to be put in a separate crate common/xxx or utils/xxx

[stdrc]

It would be better to be put in a separate crate common/xxx or utils/xxx

Which one do you mean? This PR actually moves three different things to common: Sentinelled, EstimatedBTreeMap and DeltaBTreeMap. They logically belong to different common/xxx: Sentinelled is a type wrapper to help order things with smallest and largest which always rank first/last, so moved to common/types; EstimatedBTreeMap is a estimated-size version of BTreeMap, so moved to common/estimated_size; DeltaBTreeMap is a util to help iterate over a BTreeMap and some changes on it, so moved to common/utils.

[xxchan]

I mean I don't want to the risingwave_common crate bloats. (Ideally) each utils should live in it's own crate. Especially for sth that can be widely used by other ppl without any context, (and doesn't depend on other stuff).

I think the common crate is for things that are widely used in our system (and it's specialized for our system, e.g., types, arrays, chunks).

How do you think?>

[xxchan]

I'm emphasizing organizing code in "crates", not "modules" 🤪

[stdrc]

I'm emphasizing organizing code in "crates", not "modules" 🤪

You're right. So we'd better separate many of the modules in common crate into different crates right? Will have a try tmr.

[xxchan]

So we'd better separate many of the modules in common crate into different crates right?

Yes, that's what in my head. Although I don't think it makes much difference for the common crate NOW (Currently it takes only a few secs to compile), I view it as a mindset problem. We can do the right things at the beginning.. Other opinions are also welcomed. 🤪

[wenym1]

Rest LGTM.

[wenym1]

As our convention, the name shall be risingwave_delta_btree_map.