[GitHub] flink issue #4358: [FLINK-7068][blob] change BlobService sub-classes for per...
Github user tillrohrmann commented on the issue: https://github.com/apache/flink/pull/4358 Thanks a lot for your work and patience with me @NicoK. Changes look good to me. I've rebased it onto the latest master and once Travis gives green light, I'll merge it :-) ---
[GitHub] flink issue #4358: [FLINK-7068][blob] change BlobService sub-classes for per...
Github user NicoK commented on the issue: https://github.com/apache/flink/pull/4358 Rebased and extended the PR as requested - the last two commits contain the changes compared to the last review. I tried to clean up some of the commits for a better merge but please note that this PR also includes #4568, #4238 and #4402 fixes and commits. For some of those, changes were applied afterwards in the review process which will cause conflicts in their respective PRs that I will close (fixed after merging this PR). ---
[GitHub] flink issue #4358: [FLINK-7068][blob] change BlobService sub-classes for per...
Github user NicoK commented on the issue: https://github.com/apache/flink/pull/4358 After an offline discussion with @tillrohrmann, we agreed to have more of the transient vs. permanent BLOB handling inside the `BlobServer` by including the type into the `BlobKey`. The caches, however, remain separate (with a common base class for shared code) because of the different cache guarantees they give. In addition, we decided upon having a get-and-delete functionality for transient BLOBs (as planned by a TODO item previously). This means the following changes: - the `BlobServer` deletes transient BLOBs after they have been successfully downloaded to a BLOB cache (the client acknowledges that) - the `TransientBlobCache` and `PermanentBlobCache` differ in the functionality they provide: - `PermanentBlobCache` uses ref-counting for cleaning up its resources - `TransientBlobCache` imposes the user to manually delete requested BLOB files (from the cache!) - `TransientBlobCache` allows uploading (transient) BLOBs - `TransientBlobCache` allows non-job-related BLOBs - both have a GET method: `#getTransientFile()` vs. `#getPermanentFile()` - irrespective of the actual cache being used, the `BlobServer` decides whether to delete the BLOB (transient BLOBs only) or not (permanent BLOBs) - theoretically, a permanent BLOB could also be retrieved from the `TransientBlobCache` (and the other way around if job-related) - this behaviour is not used though but basic tests exist to cover it ---
[GitHub] flink issue #4358: [FLINK-7068][blob] change BlobService sub-classes for per...
Github user NicoK commented on the issue: https://github.com/apache/flink/pull/4358 sorry for the mess, but let me also drag in #4568 and adapt the code in here (which is moved from `BlobCache` to `PermanentBlobCache` by this PR) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #4358: [FLINK-7068][blob] change BlobService sub-classes for per...
Github user NicoK commented on the issue: https://github.com/apache/flink/pull/4358 Rebased onto `master`, but had to drag in #4402 early to fix the end-to-end tests failing due to spurious warnings. The test failure you observed was actually a test instability introduced with #4238 for which I added a hotfix to this PR. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #4358: [FLINK-7068][blob] change BlobService sub-classes for per...
Github user tillrohrmann commented on the issue: https://github.com/apache/flink/pull/4358 I've merged #4238. Could you please rebase onto the latest master? Moreover, I've noticed that `JobManagerCleanupITCase.testBlobServerCleanupCancelledJob` failed in one of the Travis runs. Not sure if this is related. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink issue #4358: [FLINK-7068][blob] change BlobService sub-classes for per...
Github user tillrohrmann commented on the issue: https://github.com/apache/flink/pull/4358 Hi Nico, I'm still waiting for Travis before merging FLINK-7056. In the meantime could you rebase this PR and squash all FLINK-7068 commits into one? Thanks. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
