[
https://issues.apache.org/jira/browse/HDDS-15006?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18072675#comment-18072675
]
Tsz-wo Sze edited comment on HDDS-15006 at 4/10/26 4:41 PM:
------------------------------------------------------------
[~adoroszlai], thanks for pointing out the javadoc of
ConcurrentHashMap#compute! I keep forgetting that it just has synchronized the
bucket access (but not a non-blocking implementation which has to retry
multiple times).
{quote}The default implementation may retry these steps when multiple threads
attempt updates including potentially calling the remapping function multiple
times.
{quote}
However, the javadoc of
[ConcurrentMap.compute|https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/ConcurrentMap.html#compute-K-java.util.function.BiFunction-]
quoted above says that the remapping function can be called multiple times. I
think it is better to just not doing connection inside the remapping function.
Otherwise, it becomes a bug when changing to other ConcurrentMap implementation.
Also, it is more efficient to make the remapping function light weighted so it
won't hold the lock for a long time. It is generally a good practices as method
in
[ConcurrentHashMap.compute|https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/ConcurrentHashMap.html#compute-K-java.util.function.BiFunction-].
{quote}... so the computation should be short and simple ...
{quote}
Let me revise this JIRA.
was (Author: szetszwo):
[~adoroszlai], thanks for pointing out the javadoc of
ConcurrentHashMap#compute! I keep forgetting that it just has synchronized the
bucket access (but not a non-blocking implementation).
{quote}The default implementation may retry these steps when multiple threads
attempt updates including potentially calling the remapping function multiple
times.
{quote}
However, the javadoc of
[ConcurrentMap.compute|https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/ConcurrentMap.html#compute-K-java.util.function.BiFunction-]
quoted above says that the remapping function can be called multiple times. I
think it is better to just not doing connection inside the remapping function.
Otherwise, it becomes a bug when changing to other ConcurrentMap implementation.
Also, it is more efficient to make the remapping function light weighted so it
won't hold the lock for a long time. It is generally a good practices as method
in
[ConcurrentHashMap.compute|https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/ConcurrentHashMap.html#compute-K-java.util.function.BiFunction-].
{quote}... so the computation should be short and simple ...
{quote}
Let me revise this JIRA.
> XceiverClientGrpc may create and drop connections without closing it
> --------------------------------------------------------------------
>
> Key: HDDS-15006
> URL: https://issues.apache.org/jira/browse/HDDS-15006
> Project: Apache Ozone
> Issue Type: Bug
> Components: Ozone Client
> Reporter: Tsz-wo Sze
> Assignee: Rishabh Patel
> Priority: Major
>
> In HDDS-14571, it uses ConcurrentHashMap.compute(..) to create connection in
> XceiverClientGrpc. However, ConcurrentHashMap.compute(..) can use the
> remappingFunction to create and drop objects when there is a race condition.
> As a result, it may create a connection and then drop it without closing it.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
