kennknowles commented on code in PR #33549:
URL: https://github.com/apache/beam/pull/33549#discussion_r1909192799
##########
sdks/java/io/iceberg/src/main/java/org/apache/beam/sdk/io/iceberg/RecordWriterManager.java:
##########
@@ -154,12 +156,12 @@ class DestinationState {
* can't create a new writer, the {@link Record} is rejected and {@code
false} is returned.
*/
boolean write(Record record) {
- partitionKey.partition(getPartitionableRecord(record));
+ routingPartitionKey.partition(getPartitionableRecord(record));
- if (!writers.asMap().containsKey(partitionKey) && openWriters >=
maxNumWriters) {
+ if (!writers.asMap().containsKey(routingPartitionKey) && openWriters >=
maxNumWriters) {
Review Comment:
nit: You aren't guaranteed that `asMap` is cheap. It could be linear time to
convert from some other underlying data structure. It is better to call `get`
and check if the result was `null`. (since this is not a `LoadingCache` you
don't have to worry about accidentally creating too many entries)
(maybe think about it for future changes)
--
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]
