Disable StorageNamespaceIndex feature gate when BtreeWatchCache enabled and deprecate it#129933
Conversation
|
This issue is currently awaiting triage. If a SIG or subproject determines this is a relevant issue, they will accept it by applying the The DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
5ede387 to
28adb5f
Compare
28adb5f to
9eea5fc
Compare
9eea5fc to
fbcae06
Compare
…abled and deprecate it Previously, the cache used a map keyed by the full object key, requiring iteration and filtering by namespace for namespace-scoped requests. This index allowed for faster responses by avoiding this iteration. With the introduction of the BtreeWatchCache, this optimization is no longer necessary. The B-tree structure allows efficient prefix-based searches, including fetching objects by namespace. Furthermore, the B-tree returns elements ordered by key, eliminating the need for separate sorting. Performance improvements with the BtreeWatchCache have been validated through benchmarks matching K8s scalability dimentions (see table below). These results demonstrate that the B-tree approach provides comparable or better performance than the map with index. Therefore, the StorageNamespaceIndex feature flag can be safely flipped to false and subsequently deprecated. | Benchmark | Btree with Index (current) | Btree without Index | Map with Index | Map without Index (sanity check) | | --------------------------------------------------------------------------------- | -------------------------- | ---------------------- | ---------------------- | -------------------------------- | | StoreList (10k Namespaces, 150k Pods, 5k Nodes, RV=, Namespace Scope) | 20.77µs ± 10% | 20.14µs ± 13% (~0%) | 19.73µs ± 6% (~0%) | 1067.34µs ± 10% (+5037.73%) | | StoreList (10k Namespaces, 150k Pods, 5k Nodes, RV=NotOlderThan, Namespace Scope) | 3.943µs ± 6% | 3.928µs ± 6% (~0%) | 3.665µs ± 3% (-7.05%) | 944.641µs ± 1% (+23857.41%) | | StoreList (50 Namespaces, 150k Pods, 5k Nodes, RV=, Namespace Scope) | 303.3µs ± 2% | 258.2µs ± 2% (-14.85%) | 340.1µs ± 3% (+12.15%) | 1668.6µs ± 4% (+450.23%) | | StoreList (50 Namespaces, 150k Pods, 5k Nodes, RV=NotOlderThan, Namespace Scope) | 286.2µs ± 3% | 234.7µs ± 1% (-17.99%) | 326.9µs ± 2% (+14.22%) | 1347.7µs ± 4% (+370.91%) | | StoreList (100 Namespaces, 110k Pods, 1k Nodes, RV=, Namespace Scope) | 125.3µs ± 2% | 112.3µs ± 5% (-10.38%) | 137.5µs ± 2% (+9.81%) | 1395.1µs ± 8% (+1013.78%) | | StoreList (100 Namespaces, 110k Pods, 1k Nodes, RV=NotOlderThan, Namespace Scope) | 120.6µs ± 2% | 113.2µs ± 1% (-6.13%) | 133.8µs ± 1% (+10.92%) | 1719.1µs ± 5% (+1325.35%) | | Geometric Mean | 68.94µs | 62.73µs (-9.02%) | 72.72µs (+5.48%) | 1.326ms (+1823.40%) |
fbcae06 to
b1ad53c
Compare
|
/retest |
|
/cc |
|
/cc @wojtek-t |
|
/lgtm |
|
LGTM label has been added. DetailsGit tree hash: a838bfecbed8fd5873c59fa9afd6532ad0218109 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: serathius, wojtek-t The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Previously, the cache used a map keyed by the full object key, requiring iteration and filtering by namespace for namespace-scoped requests. This index allowed for faster responses by avoiding this iteration.
With the introduction of the BtreeWatchCache, this optimization is no longer necessary. The B-tree structure allows efficient prefix-based searches, including fetching objects by namespace.
Furthermore, the B-tree returns elements ordered by key, eliminating the need for separate sorting.
Performance improvements with the BtreeWatchCache have been validated through benchmarks matching K8s scalability dimentions (see table below). These results demonstrate that the B-tree approach provides comparable or better performance than the map with index. Therefore, the StorageNamespaceIndex feature flag can be safely flipped to false and subsequently deprecated.
/kind cleanup
Fixes #129189
/assign @deads2k @jpbetz @wojtek-t