curcur commented on a change in pull request #14943:
URL: https://github.com/apache/flink/pull/14943#discussion_r584026554
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/state/KeyedStateBackend.java
##########
@@ -136,6 +138,15 @@
*/
boolean deregisterKeySelectionListener(KeySelectionListener<K> listener);
+ /** Returns the total number of state entries across all keys/namespaces.
*/
+ @VisibleForTesting
+ int numKeyValueStateEntries();
Review comment:
That's a good question, I agree that put
`int numKeyValueStateEntries()` in `KeyedStateBackend.java` for testing
purposes do not seem elegant. But let's see what will happen if we move it to
`AbstractKeyedStateBackend`
`numKeyValueStateEntries` is used in `StateBackendTestBase`; the keyed state
backend here is a `CheckpointableKeyedStateBackend` so that we can test for
both `AbstractKeyedStateBackend` and `DelegateKeyedStateBackend`.
- The current solution is to `int numKeyValueStateEntries()` in
`KeyedStateBackend.java`
- An alternative way is to put
add `int numKeyValueStateEntries()` in `AbstractKeyedStateBackend`
add `int numKeyValueStateEntries()` in `DelegateKeyedStateBackend` (that is
an interface as well)
check whether the `CheckpointableKeyedStateBackend` is an
`AbstractKeyedStateBackend` or `DelegateKeyedStateBackend` (or add an wrap
method in KeyedStateBackend, which I do not like at all).
That's why I choose to move `int numKeyValueStateEntries()` to
`KeyedStateBackend.java`
Do you have better ways of doing it? Comparing to the current solution, I do
not like the alternative way even more.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
