You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by NicoK <gi...@git.apache.org> on 2017/06/21 13:43:24 UTC
[GitHub] flink pull request #4158: [FLINK-6916][blob] remove (unused) NAME_ADDRESSABL...
GitHub user NicoK opened a pull request:
https://github.com/apache/flink/pull/4158
[FLINK-6916][blob] remove (unused) NAME_ADDRESSABLE mode
This is based upon #4146 and in addition removes the currently unused `NAME_ADDRESSABLE` BLOB addressing mode. There are currently no plans to use it and previous attempts to do so resulted in several bugs showing up, especially regarding the cleanup of these files (see #3742, #3512).
FLIP-19 will actually also use the `CONTENT_ADDRESSABLE` mode to achieve the same and thus `NAME_ADDRESSABLE` can finally be removed.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/NicoK/flink flink-6916-remove-nameaddressable
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/4158.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #4158
----
commit ce719ee39fbbca7b7828c17d9792fc87d37450c7
Author: Nico Kruber <ni...@data-artisans.com>
Date: 2017-01-06T17:42:58Z
[FLINK-6008][docs] update some config options to the new, non-deprecated ones
commit 9efa8808e46adc1253f52a6a8cec6d3b4d29fee3
Author: Nico Kruber <ni...@data-artisans.com>
Date: 2016-12-20T15:49:57Z
[FLINK-6008][docs] minor improvements in the BlobService docs
commit ca3d533b0affa645ec93d40de378dadc829bbfe5
Author: Nico Kruber <ni...@data-artisans.com>
Date: 2016-12-20T17:27:13Z
[FLINK-6008] refactor BlobCache#getURL() for cleaner code
commit 0eededeb36dd833835753def7f4bb27c9d5fb67e
Author: Nico Kruber <ni...@data-artisans.com>
Date: 2017-03-09T17:14:02Z
[FLINK-6008] use Preconditions.checkArgument in BlobClient
commit 6249041a9db2b39ddf54e79a1aed5e7706e739c7
Author: Nico Kruber <ni...@data-artisans.com>
Date: 2016-12-21T15:23:29Z
[FLINK-6008] do not fail the BlobServer if delete fails
also extend the delete tests and remove one code duplication
commit e681239a538547f752d65358db1ebd2ba312b33c
Author: Nico Kruber <ni...@data-artisans.com>
Date: 2017-03-17T15:21:40Z
[FLINK-6008] fix concurrent job directory creation
also add according unit tests
commit 20beae2dbc91859e2ec724b35b20536dcd11fe90
Author: Nico Kruber <ni...@data-artisans.com>
Date: 2017-04-18T14:37:37Z
[FLINK-6008] some comments about BlobLibraryCacheManager cleanup
commit 8a33517fe6eb2fa932ab17cb0d82a3fa8d7b8d0b
Author: Nico Kruber <ni...@data-artisans.com>
Date: 2017-04-19T13:39:03Z
[hotfix] minor typos
commit 23889866ac21494fc4af90905ab1518cbe897118
Author: Nico Kruber <ni...@data-artisans.com>
Date: 2017-04-19T14:10:16Z
[FLINK-6008] further cleanup tests for BlobLibraryCacheManager
commit 01b1a245528c264a6061ed3a48b24c5a207369f6
Author: Nico Kruber <ni...@data-artisans.com>
Date: 2017-06-14T16:01:47Z
[FLINK-6008] do not guard a delete() call with a check for existence
commit cb249759b79d88eda37a8bb149040be3052059ac
Author: Nico Kruber <ni...@data-artisans.com>
Date: 2017-06-16T08:51:04Z
[FLINK-6916][blob] remove (unused) NAME_ADDRESSABLE mode
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #4158: [FLINK-7052][blob] remove (unused) NAME_ADDRESSABL...
Posted by NicoK <gi...@git.apache.org>.
Github user NicoK commented on a diff in the pull request:
https://github.com/apache/flink/pull/4158#discussion_r126398901
--- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/blob/BlobClient.java ---
@@ -551,33 +425,17 @@ else if (response == RETURN_ERROR) {
* @param jobID
* the ID of job the BLOB belongs to or <code>null</code> to indicate the upload of a
* content-addressable BLOB
- * @param key
- * the key of the BLOB to upload or <code>null</code> to indicate the upload of a content-addressable BLOB
* @throws IOException
* thrown if an I/O error occurs while writing the header data to the output stream
*/
- private void sendPutHeader(OutputStream outputStream, JobID jobID, String key) throws IOException {
- // sanity check that either both are null or both are not null
- if ((jobID != null || key != null) && !(jobID != null && key != null)) {
- throw new IllegalArgumentException();
- }
+ private void sendPutHeader(OutputStream outputStream, JobID jobID) throws IOException {
+ checkArgument(jobID == null);
--- End diff --
no - in this PR, the jobID is not used anymore - a future PR will add uses for that again
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #4158: [FLINK-7052][blob] remove (unused) NAME_ADDRESSABL...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/4158#discussion_r126381889
--- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/blob/BlobClient.java ---
@@ -551,33 +425,17 @@ else if (response == RETURN_ERROR) {
* @param jobID
* the ID of job the BLOB belongs to or <code>null</code> to indicate the upload of a
* content-addressable BLOB
- * @param key
- * the key of the BLOB to upload or <code>null</code> to indicate the upload of a content-addressable BLOB
* @throws IOException
* thrown if an I/O error occurs while writing the header data to the output stream
*/
- private void sendPutHeader(OutputStream outputStream, JobID jobID, String key) throws IOException {
- // sanity check that either both are null or both are not null
- if ((jobID != null || key != null) && !(jobID != null && key != null)) {
- throw new IllegalArgumentException();
- }
+ private void sendPutHeader(OutputStream outputStream, JobID jobID) throws IOException {
+ checkArgument(jobID == null);
--- End diff --
should this be `!=`?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #4158: [FLINK-7052][blob] remove (unused) NAME_ADDRESSABL...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/4158#discussion_r126382129
--- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/blob/BlobClient.java ---
@@ -587,67 +445,14 @@ private void sendPutHeader(OutputStream outputStream, JobID jobID, String key) t
/**
* Deletes the BLOB identified by the given BLOB key from the BLOB server.
*
- * @param key
+ * @param bKey
* the key to identify the BLOB
* @throws IOException
* thrown if an I/O error occurs while transferring the request to
* the BLOB server or if the BLOB server cannot delete the file
*/
- public void delete(BlobKey key) throws IOException {
- checkArgument(key != null, "BLOB key must not be null.");
-
- deleteInternal(null, null, key);
- }
-
- /**
- * Deletes the BLOB identified by the given job ID and key from the BLOB server.
- *
- * @param jobId
- * the job ID to identify the BLOB
- * @param key
- * the key to identify the BLOB
- * @throws IOException
- * thrown if an I/O error occurs while transferring the request to the BLOB server
- */
- public void delete(JobID jobId, String key) throws IOException {
- checkArgument(jobId != null, "Job id must not be null.");
- checkArgument(key != null, "BLOB name must not be null.");
-
- if (key.length() > MAX_KEY_LENGTH) {
- throw new IllegalArgumentException("Keys must not be longer than " + MAX_KEY_LENGTH);
- }
-
- deleteInternal(jobId, key, null);
- }
-
- /**
- * Deletes all BLOBs belonging to the job with the given ID from the BLOB server
- *
- * @param jobId
- * the job ID to identify the BLOBs to be deleted
- * @throws IOException
- * thrown if an I/O error occurs while transferring the request to the BLOB server
- */
- public void deleteAll(JobID jobId) throws IOException {
- checkArgument(jobId != null, "Job id must not be null.");
-
- deleteInternal(jobId, null, null);
- }
-
- /**
- * Delete one or multiple BLOBs from the BLOB server.
- *
- * @param jobId The job ID to identify the BLOB(s) to be deleted.
- * @param key The key to identify the specific BLOB to delete or <code>null</code> to delete
- * all BLOBs associated with the job id.
- * @param bKey The blob key to identify a specific content addressable BLOB. This parameter
- * is exclusive with jobId and key.
- * @throws IOException Thrown if an I/O error occurs while transferring the request to the BLOB server.
- */
- private void deleteInternal(JobID jobId, String key, BlobKey bKey) throws IOException {
- if ((jobId != null && bKey != null) || (jobId == null && bKey == null)) {
- throw new IllegalArgumentException();
- }
+ public void delete(BlobKey bKey) throws IOException {
--- End diff --
Let's call this `blobKey`. for consistency (since we're changing the name anyway)
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #4158: [FLINK-7052][blob] remove (unused) NAME_ADDRESSABLE mode
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/4158
@NicoK Could you rebase 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #4158: [FLINK-7052][blob] remove (unused) NAME_ADDRESSABL...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/4158
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #4158: [FLINK-7052][blob] remove (unused) NAME_ADDRESSABLE mode
Posted by NicoK <gi...@git.apache.org>.
Github user NicoK commented on the issue:
https://github.com/apache/flink/pull/4158
there you go
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #4158: [FLINK-7052][blob] remove (unused) NAME_ADDRESSABLE mode
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/4158
+1 to merge.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #4158: [FLINK-7052][blob] remove (unused) NAME_ADDRESSABLE mode
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/4158
merging.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---