gemini-code-assist[bot] commented on code in PR #38603:
URL: https://github.com/apache/beam/pull/38603#discussion_r3292561703
##########
sdks/java/io/solace/src/main/java/org/apache/beam/sdk/io/solace/read/UnboundedSolaceReader.java:
##########
@@ -151,30 +156,39 @@ public boolean advance() {
}
solaceOriginalRecord = receivedXmlMessage;
solaceMappedRecord =
getCurrentSource().getParseFn().apply(receivedXmlMessage);
- receivedMessages.add(receivedXmlMessage);
+ synchronized (lock) {
+ receivedMessages.add(receivedXmlMessage);
+ }
Review Comment:
The synchronization here is unnecessary and can cause minor performance
degradation. In Apache Beam, `advance()` and `getCheckpointMark()` are
guaranteed to be called by the same reader thread. Since `receivedMessages` is
only accessed by these two methods and not by the finalizer thread (which only
touches `pendingCheckpoints`), there is no cross-thread contention on this
specific queue. Removing this `synchronized` block avoids potential blocking of
the critical reader thread when the finalizer thread holds the lock during its
own (short) synchronized blocks.
```suggestion
solaceMappedRecord =
getCurrentSource().getParseFn().apply(receivedXmlMessage);
receivedMessages.add(receivedXmlMessage);
return 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]
