Skip to content

[FLINK-8559][RocksDB] Release resources if snapshot operation fails#5412

Closed
zentol wants to merge 1 commit intoapache:masterfrom
zentol:8559
Closed

[FLINK-8559][RocksDB] Release resources if snapshot operation fails#5412
zentol wants to merge 1 commit intoapache:masterfrom
zentol:8559

Conversation

@zentol
Copy link
Copy Markdown
Contributor

@zentol zentol commented Feb 5, 2018

What is the purpose of the change

This PR ensures that RocksDB resources are released if RocksDBIncrementalSnapshotOperation#takeSnapshot throws an Exception.

We now catch the exception, cancel the SnapshotOperation, and re-throw the original exception.

Verifying this change

I've verified this manually by running JobManagerHACheckpointRecoveryITCase on Windows where takeSnapshot fails due to FLINK-8557.

I couldn't come up with proper test. The method hardly does anything in the first place and every solution i could think of would depend a lot on implementation details (like mocking Checkpoint.create() to throw an exception).

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: (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)

@StefanRRichter

@StefanRRichter
Copy link
Copy Markdown
Contributor

LGTM

@zentol
Copy link
Copy Markdown
Contributor Author

zentol commented Feb 5, 2018

merging.

zentol added a commit to zentol/flink that referenced this pull request Feb 5, 2018
zentol added a commit to zentol/flink that referenced this pull request Feb 5, 2018
zentol added a commit to zentol/flink that referenced this pull request Feb 6, 2018
zentol added a commit to zentol/flink that referenced this pull request Feb 6, 2018
zentol added a commit to zentol/flink that referenced this pull request Feb 6, 2018
zentol added a commit to zentol/flink that referenced this pull request Feb 6, 2018
zentol added a commit to zentol/flink that referenced this pull request Feb 6, 2018
@asfgit asfgit closed this in dbb81ac Feb 6, 2018
@zentol zentol deleted the 8559 branch February 7, 2018 09:16
JTaky pushed a commit to criteo-forks/flink that referenced this pull request Feb 16, 2018
glaksh100 pushed a commit to lyft/flink that referenced this pull request Jun 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants