lukecwik commented on code in PR #17201:
URL: https://github.com/apache/beam/pull/17201#discussion_r845437349
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/IsmReaderImpl.java:
##########
@@ -370,7 +373,7 @@ boolean bloomFilterMightContain(RandomAccessData keyBytes) {
position(rawChannel, footer.getBloomFilterPosition());
bloomFilter =
ScalableBloomFilterCoder.of().decode(Channels.newInputStream(rawChannel));
- indexPerShard = new HashMap<>();
+ indexPerShard = new ConcurrentHashMap<>();
Review Comment:
Note that initializeBloomFilterAndIndexPerShard is synchronized which is the
only method that mutates the indexPerShard variable. Since
initializeBloomFilterAndIndexPerShard is invoked within overKeyComponents that
means there is a memory barrier between the thread that created the
indexPerShard and other threads ensuring that the object is safely published
across threads.
Also, the [Javadoc for
HashMap](https://docs.oracle.com/javase/8/docs/api/java/util/HashMap.html) says
that concurrent reads are ok, its concurrent modifications that require
synchronization:
"If multiple threads access a hash map concurrently, and at least one of the
threads modifies the map structurally, it must be synchronized externally."
--
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: github-unsubscr...@beam.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
