[Java] Disable soft delete policy when creating a default bucket for a project. #31324
Conversation
Also, getBucket() and removeBucket() are added into GcsUtil.
8452bfd
to
8f2d2c6
Compare
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control |
|
|
||
| String tempLocation = | ||
| GcpOptions.GcpTempLocationFactory.tryCreateDefaultBucketWithPrefix( | ||
| options, crmClient, "gcp-options-it-"); |
There was a problem hiding this comment.
Should we check for existence of the bucket and delete it if it exists before we do this? That way we avoid a bad condition where the delete fails on one test and then we're stuck in a bad state indefinitely
There was a problem hiding this comment.
Can we remove it? I haven't checked the internal logic. Could two Dataflow jobs use the same default bucket?
There was a problem hiding this comment.
The default bucket name is a concatenation of a fixed prefix, region and a project number. So for a given project, if it is running in the same region, the default bucket name would be the same.
For our integration test, it would always be the same as "gcp-options-it-us-central1-844138762903". Actually, this may cause race condition if we run this integration test twice at the same time though.
There was a problem hiding this comment.
For this test, I think I have to add some random number in the prefix then.
There was a problem hiding this comment.
Should we check for existence of the bucket and delete it if it exists before we do this? That way we avoid a bad condition where the delete fails on one test and then we're stuck in a bad state indefinitely
Well, if we add a random number to avoid race condition, then we could end up with multiple dangling buckets if deletion fails. That's a tradeoff we need to make, to avoid race condition or to avoid dangling buckets. Thoughts?
There was a problem hiding this comment.
I think that is fine. If it causes problems we could always add a cleanup job like https://github.com/apache/beam/blob/master/.test-infra/tools/stale_bq_datasets_cleaner.sh
There was a problem hiding this comment.
Should we check for existence of the bucket and delete it if it exists before we do this? That way we avoid a bad condition where the delete fails on one test and then we're stuck in a bad state indefinitely
Done.
|
Minor: can we update the changes.md regarding to this fix? |
62c7167
to
266f63f
Compare
266f63f
to
6601e2c
Compare
addresses #31330 in Java SDK