[FLINK-38225] Sets proper precondition to ensure that GCS operations are retryable

[JTaky]

What is the purpose of the change

Ensures that the operations become idempotent or atomic, allowing the GCS client to safely retry the 503 errors. Thus client can actually perform the retry in such conditions.

Original retry logic was added in #24753.

Thanks Xi Zhang for pointing to the problem and providing the solution in https://issues.apache.org/jira/browse/FLINK-38225.

Brief change log

• Adds an option when composing the GCS request, to ensure that operation is atomic
• Adds an option when writing to GCS, to ensure that write operation is atomic

Verifying this change

This change is already covered by existing tests, such as GSRecoverableWriterTest

Does this pull request potentially affect one of the following parts:

• Dependencies (does it add or upgrade a dependency): no
• The public API, i.e., is any changed class annotated with @public(Evolving): no
• The serializers: no
• The runtime per-record code paths (performance sensitive): no
• Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: no
• The S3 file system connector: no

Documentation

• Does this pull request introduce a new feature? no
• If yes, how is the feature documented? not applicable

[flinkbot]

CI report:

• 56f84ba Azure: SUCCESS

Bot commands

The @flinkbot bot supports the following commands:

• @flinkbot run azure re-run the last Azure build

[davidradl]

is there a way to junit this change?

[JTaky]

We can wrap the storage in the UT and ensure that proper option is passed.

For this we need to extend the Storage. Which is for InternalExtensionOnly.

Some e2e would be great to have, but from what I understand only Azure is available in CI.

[JTaky]

Tried briefly to make some UT and without mocking it is hard. Currently GSBlobStorageImpl is too low level and it is the component which is being mocked in the codebase.

Creating a TestStorage to catch GCP calls is also hard, notable because some of the methods returns Blob object, which cannot be created without reflections.

[JTaky]

@davidradl thanks for challenging. I have reconsidered approach and added some tests, for functionality. As result the new test dependency to emulate GS Storage is added.
This test dependency doesn't allow to use versioning, so I cannot test the atomicity of the operations, meanwhile happy paths are added to test write and compose methods.

[xintongsong]

LGTM

[xizzzz]

@JTaky thanks for the fix!

[Hang-Datadome]

got the same issue here, Thank you guys for the fix!