[
https://issues.apache.org/jira/browse/RATIS-1923?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17783877#comment-17783877
]
Xinyu Tan commented on RATIS-1923:
----------------------------------
ConcurrentHashMap uses cas and synchronized to preempt the corresponding node.
Once preempted, it uses synchronized to execute the mappingFunction. So the
atomicity here should guarantee that the mappingFunction will only be executed
once. When a concurrent request comes in, it will see that the key already has
a value, so the mappingFunction will not be evaluated again
> Netty: atomic operations require side-effect-free functions
> -----------------------------------------------------------
>
> Key: RATIS-1923
> URL: https://issues.apache.org/jira/browse/RATIS-1923
> Project: Ratis
> Issue Type: Bug
> Components: Netty
> Reporter: Tsz-wo Sze
> Assignee: Tsz-wo Sze
> Priority: Major
> Fix For: 3.0.0
>
> Attachments: for-loop.jpg, screenshot-1.png
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> - The update methods in AtomicReference run in a loop and may call the update
> function multiple time. The old values created by the update function may be
> discarded without cleaning up. In NettyClientStreamRpc.Connection the connect
> method is not side-effect-free. Although it passes a [MemoizedSupplier to
> getAndUpdate|https://github.com/apache/ratis/blob/47e5b6aa148cfe777fb3b7742154fe6b417ea42a/ratis-netty/src/main/java/org/apache/ratis/netty/client/NettyClientStreamRpc.java#L187-L188],
> it still is possible that a value is supplied but then discarded in the
> AtomicReference loop.
> - --Similarly, the computeIfAbsent in ConcurrentHashMap also runs in a loop
> and may call the mapping function multiple times. DataStreamMap is
> implemented using a ConcurrentHashMap. However, [the mapping function is not
> side-effect-free|https://github.com/apache/ratis/blob/47e5b6aa148cfe777fb3b7742154fe6b417ea42a/ratis-netty/src/main/java/org/apache/ratis/netty/server/DataStreamManagement.java#L281-L289]
> although it uses a MemoizedSupplier.--
> (Update: [~tanxinyu] pointed out that this was not a problem. Thanks! The
> computeIfAbsent method does not require side-effect-free as described in the
> [javadoc|https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/ConcurrentHashMap.html#computeIfAbsent-K-java.util.function.Function-];
> see also the comments below.)
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
