gemini-code-assist[bot] commented on code in PR #37691:
URL: https://github.com/apache/beam/pull/37691#discussion_r2843278955
##########
sdks/java/harness/src/main/java/org/apache/beam/fn/harness/state/OrderedListUserState.java:
##########
@@ -161,7 +162,9 @@ public OrderedListUserState(
BeamFnStateClient beamFnStateClient,
String instructionId,
StateKey stateKey,
- Coder<T> valueCoder) {
+ Coder<T> valueCoder,
+ boolean hasNoState,
+ boolean onlyBundleForKeys) {
Review Comment:
The `onlyBundleForKeys` parameter is stored but appears to be unused in
`asyncClose` to skip `StateAppendRequest`s. This seems to be an omission, as
`BagUserState` uses this flag to optimize writes. Please ensure this flag is
used to skip appending state when it's true.
##########
sdks/java/harness/src/main/java/org/apache/beam/fn/harness/state/StateFetchingIterators.java:
##########
@@ -328,16 +330,19 @@ public static <T> Block<T> fromValues(
private final BeamFnStateClient beamFnStateClient;
private final StateRequest stateRequestForFirstChunk;
private final Coder<T> valueCoder;
+ private final boolean hasNoState;
public CachingStateIterable(
Cache<IterableCacheKey, Blocks<T>> cache,
BeamFnStateClient beamFnStateClient,
StateRequest stateRequestForFirstChunk,
- Coder<T> valueCoder) {
+ Coder<T> valueCoder,
+ boolean hasNoState) {
this.cache = cache;
this.beamFnStateClient = beamFnStateClient;
this.stateRequestForFirstChunk = stateRequestForFirstChunk;
this.valueCoder = valueCoder;
+ this.hasNoState = hasNoState;
Review Comment:
The `hasNoState` field is correctly stored here, but it seems it's not being
passed to the `LazyBlockingStateFetchingIterator` within the inner class
`CachingStateIterator`. The `CachingStateIterator` constructor should be
updated to use the new `LazyBlockingStateFetchingIterator` constructor that
accepts `hasNoState`. Without this change, state fetches won't be skipped even
when `hasNoState` is true.
##########
sdks/java/harness/src/main/java/org/apache/beam/fn/harness/state/MultimapUserState.java:
##########
@@ -84,7 +86,9 @@ public MultimapUserState(
String instructionId,
StateKey stateKey,
Coder<K> mapKeyCoder,
- Coder<V> valueCoder) {
+ Coder<V> valueCoder,
+ boolean hasNoState,
+ boolean onlyBundleForKeys) {
Review Comment:
The `onlyBundleForKeys` parameter is stored but appears to be unused in
`asyncClose` to skip `StateAppendRequest`s. This seems to be an omission, as
`BagUserState` uses this flag to optimize writes. Please ensure this flag is
used to skip appending state when it's true.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
