Jackie-Jiang commented on code in PR #19115: URL: https://github.com/apache/pinot/pull/19115#discussion_r3692947396
########## pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/PartialUpsertHandler.java: ########## @@ -42,6 +43,10 @@ /// /// It is also possible to define a custom logic for merging rows by implementing [PartialUpsertMerger]. /// If a merger for row is defined then it takes precedence and ignores column mergers. +/// +/// NOTE: This class is not thread safe. The post partial upsert transformers keep reusable evaluation state, so a +/// handler belongs to exactly one partition and must be used by one thread at a time. +@NotThreadSafe Review Comment: Let's add `synchronized` to `merge()` and mark this class `@ThreadSafe`. Given the expected single thread access pattern + the size of `merge()` method, it won't add any noticeable overhead for sure -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
