You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "ahmedabu98 (via GitHub)" <gi...@apache.org> on 2025/01/09 16:16:36 UTC
[PR] [Managed Iceberg] Fix partition value race condition [beam]
ahmedabu98 opened a new pull request, #33549:
URL: https://github.com/apache/beam/pull/33549
Fixes #33497 (see [comment](https://github.com/apache/beam/issues/33497#issuecomment-2580542191) and [comment](https://github.com/apache/beam/issues/33497#issuecomment-2580614117) for details)
Also updates tests
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Managed Iceberg] Fix partition value race condition [beam]
Posted by "ahmedabu98 (via GitHub)" <gi...@apache.org>.
ahmedabu98 commented on code in PR #33549:
URL: https://github.com/apache/beam/pull/33549#discussion_r1909198429
##########
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:
Thanks for the tip, will do
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Managed Iceberg] Fix partition value race condition [beam]
Posted by "kennknowles (via GitHub)" <gi...@apache.org>.
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Managed Iceberg] Fix partition value race condition [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #33549:
URL: https://github.com/apache/beam/pull/33549#issuecomment-2580953622
Assigning reviewers. If you would like to opt out of this review, comment `assign to next reviewer`:
R: @damondouglas for label java.
R: @Abacn for label build.
R: @johnjcasey for label io.
Available commands:
- `stop reviewer notifications` - opt out of the automated review tooling
- `remind me after tests pass` - tag the comment author after tests pass
- `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)
The PR bot will only process comments in the main thread (not review comments).
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Managed Iceberg] Fix partition value race condition [beam]
Posted by "ahmedabu98 (via GitHub)" <gi...@apache.org>.
ahmedabu98 merged PR #33549:
URL: https://github.com/apache/beam/pull/33549
--
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-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org