acrites commented on code in PR #40750:
URL: https://github.com/apache/beam/pull/40750#discussion_r4391873031
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/windmill/state/WindmillMapViaMultimap.java:
##########
@@ -62,19 +62,26 @@ public void put(KeyT key, ValueT value) {
multimap.put(key, value);
}
+ @SuppressWarnings("nullness")
@Override
public ReadableState<ValueT> computeIfAbsent(
KeyT key, Function<? super KeyT, ? extends ValueT> mappingFunction) {
// Note that computeIfAbsent comments indicate that the read is lazy but
this matches the
- // existing eager
- // behavior of WindmillMap.
+ // existing eager behavior of WindmillMap.
Iterable<ValueT> existingValues = multimap.get(key).read();
- if (Iterables.isEmpty(existingValues)) {
+ Iterator<ValueT> iterator = existingValues.iterator();
+ if (!iterator.hasNext()) {
ValueT inserted = mappingFunction.apply(key);
multimap.put(key, inserted);
return ReadableStates.immediate(inserted);
} else {
- return
ReadableStates.immediate(Iterables.getOnlyElement(existingValues));
+ ValueT existing = Iterators.getOnlyElement(iterator);
+ if (existing == null) {
Review Comment:
In all of these cases the behavior seems to be "if the value is currently
mapped to null, apply the mapping function and overwrite it with whatever the
mapping function returns". I think they all return the previous value that was
stored in the map. So it seems like `WindmillMapViaMultimap` is the only class
not implementing `computeIfAbsent` based on the original comments.
--
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]