[GitHub] [flink] StephanEwen opened pull request #6604: [FLINK-9061] Optionally add entropy to checkpoint paths better S3 scalability

["StephanEwen (GitHub)" <gi...@apache.org>]

## What is the purpose of the change

This pull request adds hooks to optionally inject entropy to the checkpoint path based upon a user defined pattern in the configuration for better S3 scalability.

This is a revised version of #6302 by @indrc (based on Flink 1.4) that ports the changes to the new FileStateStorage code introduced in Flink 1.5.

## Brief change log

  - Adds an optional `CheckpointPathFilter` that is applied to data and metadata paths before creating the files
  - Adds an implementation of that filter that replaces entropy keys with entropy (data files) or removes the entropy key (metadata files)
  - Adds functionality to read the entropy pattern from the config or set it on the state backend, and configure the CheckpointPathFilter based on that

## Verifying this change

  - Added unit tests under `org.apache.flink.runtime.state.filesystem` 

## 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, Yarn/Mesos, ZooKeeper: **yes**
  - The S3 file system connector: **no**

## Documentation

  - Does this pull request introduce a new feature? **yes**
  - If yes, how is the feature documented? **docs**


[ Full content available at: https://github.com/apache/flink/pull/6604 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org

[GitHub] [flink] StephanEwen commented on issue #6604: [FLINK-9061] Optionally add entropy to checkpoint paths better S3 scalability

["StephanEwen (GitHub)" <gi...@apache.org>]

Test failures fixed - needed to adjust the mocking in some other tests. Classic ;-)

[ Full content available at: https://github.com/apache/flink/pull/6604 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org

[GitHub] [flink] StephanEwen closed pull request #6604: [FLINK-9061] Optionally add entropy to checkpoint paths better S3 scalability

["StephanEwen (GitHub)" <gi...@apache.org>]

[ pull request closed by StephanEwen ]

[ Full content available at: https://github.com/apache/flink/pull/6604 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org

[GitHub] [flink] StephanEwen commented on issue #6604: [FLINK-9061] Optionally add entropy to checkpoint paths better S3 scalability

["StephanEwen (GitHub)" <gi...@apache.org>]

Okay, updated the implementation. Moved the entropy injection code tho the FS implementation, which is much more clean.

PR description updated...

[ Full content available at: https://github.com/apache/flink/pull/6604 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org

[GitHub] [flink] StefanRRichter commented on issue #6604: [FLINK-9061] Optionally add entropy to checkpoint paths better S3 scalability

["StefanRRichter (GitHub)" <gi...@apache.org>]

Modulo some test failures from NPEs.

[ Full content available at: https://github.com/apache/flink/pull/6604 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org

[GitHub] [flink] StefanRRichter commented on pull request #6604: [FLINK-9061] Optionally add entropy to checkpoint paths better S3 scalability

["StefanRRichter (GitHub)" <gi...@apache.org>]

Isn't this problematic here: if we have a concept that paths can contain an entropy key, we still call `mkdirs` on the "unfiltered" paths in the following lines. It seems to clash a bit with the storage location idea. 

[ Full content available at: https://github.com/apache/flink/pull/6604 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org

[GitHub] [flink] StefanRRichter commented on pull request #6604: [FLINK-9061] Optionally add entropy to checkpoint paths better S3 scalability

["StefanRRichter (GitHub)" <gi...@apache.org>]

`PathTransformer` might be a better name for what it does (see your own comment).

I also wonder if we could simply collapse the separation path + transformer into a `PathProvider` or `PathFactory` and we could have one for data and one for meta data. This can be a bit tricky for `FsCheckpointStorageLocation`, but also the current solution seems problematic.

[ Full content available at: https://github.com/apache/flink/pull/6604 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org

[GitHub] [flink] StephanEwen commented on issue #6604: [FLINK-9061] Optionally add entropy to checkpoint paths better S3 scalability

["StephanEwen (GitHub)" <gi...@apache.org>]

Fixed for 1.6 in a58fa985c4944dd0cf39fa622ec0aa4b35f21f44

For master, I will open a new PR that is rebased on the latest commits...

[ Full content available at: https://github.com/apache/flink/pull/6604 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org

[GitHub] [flink] StefanRRichter commented on issue #6604: [FLINK-9061] Optionally add entropy to checkpoint paths better S3 scalability

["StefanRRichter (GitHub)" <gi...@apache.org>]

👍  from my side in case you were waiting for that ;-)

[ Full content available at: https://github.com/apache/flink/pull/6604 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org

[GitHub] [flink] StephanEwen commented on issue #6604: [FLINK-9061] Optionally add entropy to checkpoint paths better S3 scalability

["StephanEwen (GitHub)" <gi...@apache.org>]

Good points - these were actually problems in the original PR as well...
Will try to come up with a solution for that...

[ Full content available at: https://github.com/apache/flink/pull/6604 ]
This message was relayed via gitbox.apache.org for devnull@infra.apache.org