curcur commented on a change in pull request #15200:
URL: https://github.com/apache/flink/pull/15200#discussion_r603092708
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/state/changelog/StateChangelogWriterFactory.java
##########
@@ -30,7 +29,7 @@
public interface StateChangelogWriterFactory<Handle extends
StateChangelogHandle<?>>
extends AutoCloseable {
- StateChangelogWriter<Handle> createWriter(OperatorID operatorID,
KeyGroupRange keyGroupRange);
+ StateChangelogWriter<Handle> createWriter(String operatorID, KeyGroupRange
keyGroupRange);
Review comment:
I think they are mostly equivalent, why do we want to change from
`OperatorID` to `String`?
##########
File path:
flink-state-backends/flink-statebackend-changelog/src/main/java/org/apache/flink/state/changelog/AbstractChangelogState.java
##########
@@ -34,10 +34,13 @@
abstract class AbstractChangelogState<K, N, V, S extends InternalKvState<K, N,
V>>
implements InternalKvState<K, N, V> {
protected final S delegatedState;
+ protected N currentNamespace;
Review comment:
`currentNamespace` is not initialized, so it is possible to be null?
From the context, it seems namespace can not be null, so it is better to
check its non-null when using it?
`checkNotNull(namespace, "Namespace");` in
`AbstractKeyedStateBackend#getPartitionedState`
##########
File path:
flink-state-backends/flink-statebackend-changelog/src/main/java/org/apache/flink/state/changelog/AbstractChangelogState.java
##########
@@ -34,10 +34,13 @@
abstract class AbstractChangelogState<K, N, V, S extends InternalKvState<K, N,
V>>
implements InternalKvState<K, N, V> {
protected final S delegatedState;
+ protected N currentNamespace;
+ protected KvStateChangeLogger<V, N> changeLogger;
- AbstractChangelogState(S state) {
+ AbstractChangelogState(S state, KvStateChangeLogger<V, N> changeLogger) {
Review comment:
@Nonnull for KvStateChangeLogger?
##########
File path:
flink-state-backends/flink-statebackend-changelog/src/main/java/org/apache/flink/state/changelog/AbstractChangelogState.java
##########
@@ -34,10 +34,13 @@
abstract class AbstractChangelogState<K, N, V, S extends InternalKvState<K, N,
V>>
implements InternalKvState<K, N, V> {
protected final S delegatedState;
+ protected N currentNamespace;
+ protected KvStateChangeLogger<V, N> changeLogger;
Review comment:
`final` ?
--
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:
[email protected]
