Skip to content

[FLINK-3556] [runtime] Remove false check in HA blob store configuration#1749

Closed
uce wants to merge 2 commits into
apache:masterfrom
uce:3556-blob
Closed

[FLINK-3556] [runtime] Remove false check in HA blob store configuration#1749
uce wants to merge 2 commits into
apache:masterfrom
uce:3556-blob

Conversation

@uce

@uce uce commented Mar 1, 2016

Copy link
Copy Markdown
Contributor

If you use the RocksDBStateBackend and configure HA w/o setting the state backend to filesystem, restoring your job will fail, because the BLOBs are not persisted. The really ugly part is that this does not log a warning or anything. I'm leaning towards making this a release blocker, but it can also be postponed to 1.0.1, if that follows very soon.

@tillrohrmann

Copy link
Copy Markdown
Contributor

Changes look good to me. Good catch @uce :-) +1 for merging.

One minor thing: We check that the configuration contains the ZOOKEEPER_RECOVERY_PATH and then give the configuration to the FileSystemBlobStore. Wouldn't it make more sense to directly pass in the value of ZOOKEEPER_RECOVERY_PATH or do the checking of the correctness of the value in the FileSystemBlobStore? If we pass the configuration, then we don't actually know, which values will be used internally. This would untie the BlobServer and the FileSystemBlobStore a bit.

@uce

uce commented Mar 1, 2016

Copy link
Copy Markdown
Contributor Author

Thanks Till! The check was duplicated in BlobServer and FileSystemBlobStore. Now only config is passed and it's the responsibility of FileSystemBlobStore to construct itself correctly or fail.

@uce

uce commented Mar 1, 2016

Copy link
Copy Markdown
Contributor Author

PS I had a local Travis build with this (1st commit), which succeeded https://travis-ci.org/uce/flink/builds/112830163 and the blob and recovery tests succeed on my local laptop with the latest commit.

@tillrohrmann

Copy link
Copy Markdown
Contributor

+1 for merging

@uce

uce commented Mar 1, 2016

Copy link
Copy Markdown
Contributor Author

OK, going to merge to master and release-1.0.

@asfgit asfgit closed this in c8f81ba Mar 1, 2016
asfgit pushed a commit that referenced this pull request Mar 1, 2016
subhankarb pushed a commit to subhankarb/flink that referenced this pull request Mar 17, 2016
@uce uce deleted the 3556-blob branch April 20, 2016 13:16
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