You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by zentol <gi...@git.apache.org> on 2017/05/10 14:28:58 UTC
[GitHub] flink pull request #3865: [FLINK-6518] Port blobserver config parameters to ...
GitHub user zentol opened a pull request:
https://github.com/apache/flink/pull/3865
[FLINK-6518] Port blobserver config parameters to ConfigOptions
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zentol/flink 6518_config_blobserver
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/3865.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 #3865
----
commit 2672f054faacb82e1a5782293e728ec3a4ce8bb6
Author: zentol <ch...@apache.org>
Date: 2017-05-10T08:26:19Z
BlobServerOptions#STORAGE_DIRECTORY
commit 2d99f91db2f9f6d3e6ef27286148192722285253
Author: zentol <ch...@apache.org>
Date: 2017-05-10T08:31:03Z
BlobServerOptions#FETCH_RETRIES
commit d0c00581e92876f9fa6fdc940e7103f61ad29e22
Author: zentol <ch...@apache.org>
Date: 2017-05-10T08:35:33Z
BlobServerOptions#FETCH_CONCURRENT
commit d3de86b8c388f9531a2ec42a3603e5eb507a0102
Author: zentol <ch...@apache.org>
Date: 2017-05-10T08:38:09Z
BlobServerOptions#FETCH_BACKLOG
commit 3d6bf9b92ba355610fdb4b0b67ffa938a8df3af3
Author: zentol <ch...@apache.org>
Date: 2017-05-10T08:41:20Z
BlobServerOptions#PORT
commit c2bbd88301cb2c759a5fef2ee43c98fa77e13426
Author: zentol <ch...@apache.org>
Date: 2017-05-10T08:45:22Z
BlobServerOptions#SSL_ENABLED
----
---
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 #3865: [FLINK-6518] Port blobserver config parameters to ...
Posted by greghogan <gi...@git.apache.org>.
Github user greghogan commented on a diff in the pull request:
https://github.com/apache/flink/pull/3865#discussion_r116233068
--- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/blob/BlobServer.java ---
@@ -111,34 +111,32 @@ private BlobServer(Configuration config, BlobStore blobStore) throws IOException
this.blobStore = checkNotNull(blobStore);
// configure and create the storage directory
- String storageDirectory = config.getString(ConfigConstants.BLOB_STORAGE_DIRECTORY_KEY, null);
+ String storageDirectory = config.getString(BlobServerOptions.STORAGE_DIRECTORY);
this.storageDir = BlobUtils.initStorageDirectory(storageDirectory);
LOG.info("Created BLOB server storage directory {}", storageDir);
// configure the maximum number of concurrent connections
- final int maxConnections = config.getInteger(
- ConfigConstants.BLOB_FETCH_CONCURRENT_KEY, ConfigConstants.DEFAULT_BLOB_FETCH_CONCURRENT);
+ final int maxConnections = config.getInteger(BlobServerOptions.FETCH_CONCURRENT);
if (maxConnections >= 1) {
--- End diff --
I had only meant to suggest this as a follow-up.
---
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 #3865: [FLINK-6518] Port blobserver config parameters to ConfigO...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/3865
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.
---
[GitHub] flink pull request #3865: [FLINK-6518] Port blobserver config parameters to ...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/3865#discussion_r116229431
--- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/blob/BlobServer.java ---
@@ -111,34 +111,32 @@ private BlobServer(Configuration config, BlobStore blobStore) throws IOException
this.blobStore = checkNotNull(blobStore);
// configure and create the storage directory
- String storageDirectory = config.getString(ConfigConstants.BLOB_STORAGE_DIRECTORY_KEY, null);
+ String storageDirectory = config.getString(BlobServerOptions.STORAGE_DIRECTORY);
this.storageDir = BlobUtils.initStorageDirectory(storageDirectory);
LOG.info("Created BLOB server storage directory {}", storageDir);
// configure the maximum number of concurrent connections
- final int maxConnections = config.getInteger(
- ConfigConstants.BLOB_FETCH_CONCURRENT_KEY, ConfigConstants.DEFAULT_BLOB_FETCH_CONCURRENT);
+ final int maxConnections = config.getInteger(BlobServerOptions.FETCH_CONCURRENT);
if (maxConnections >= 1) {
--- End diff --
I would be in favor of that. This would also be very useful for options that essentially work on enums, like `HAOptions#HA_MODE` which only allows `NONE` and `ZOOKEEPER`.
---
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 #3865: [FLINK-6518] Port blobserver config parameters to ...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/3865#discussion_r116231353
--- Diff: flink-core/src/main/java/org/apache/flink/configuration/BlobServerOptions.java ---
@@ -0,0 +1,76 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.flink.configuration;
+
+import org.apache.flink.annotation.PublicEvolving;
+
+import static org.apache.flink.configuration.ConfigOptions.key;
+
+/**
+ * Configuration options for the BlobServer.
+ */
+@PublicEvolving
--- End diff --
The downgrading of to `@PublicEvolving` seems like a common thing for the `ConfigOptions`, as all of them are annotated as such.
We definitely need some kind of evaluation to promote `@PublicEvolving` to `@Public`. I'm actually not sure if we have ever promoted something to being `@Public`.
---
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 #3865: [FLINK-6518] Port blobserver config parameters to ...
Posted by greghogan <gi...@git.apache.org>.
Github user greghogan commented on a diff in the pull request:
https://github.com/apache/flink/pull/3865#discussion_r116224135
--- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/blob/BlobServer.java ---
@@ -111,34 +111,32 @@ private BlobServer(Configuration config, BlobStore blobStore) throws IOException
this.blobStore = checkNotNull(blobStore);
// configure and create the storage directory
- String storageDirectory = config.getString(ConfigConstants.BLOB_STORAGE_DIRECTORY_KEY, null);
+ String storageDirectory = config.getString(BlobServerOptions.STORAGE_DIRECTORY);
this.storageDir = BlobUtils.initStorageDirectory(storageDirectory);
LOG.info("Created BLOB server storage directory {}", storageDir);
// configure the maximum number of concurrent connections
- final int maxConnections = config.getInteger(
- ConfigConstants.BLOB_FETCH_CONCURRENT_KEY, ConfigConstants.DEFAULT_BLOB_FETCH_CONCURRENT);
+ final int maxConnections = config.getInteger(BlobServerOptions.FETCH_CONCURRENT);
if (maxConnections >= 1) {
--- End diff --
Do you think we should refactor some bounds checking and warning into `ConfigOptions`?
---
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 #3865: [FLINK-6518] Port blobserver config parameters to ...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/3865#discussion_r116231615
--- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/blob/BlobServer.java ---
@@ -111,34 +111,32 @@ private BlobServer(Configuration config, BlobStore blobStore) throws IOException
this.blobStore = checkNotNull(blobStore);
// configure and create the storage directory
- String storageDirectory = config.getString(ConfigConstants.BLOB_STORAGE_DIRECTORY_KEY, null);
+ String storageDirectory = config.getString(BlobServerOptions.STORAGE_DIRECTORY);
this.storageDir = BlobUtils.initStorageDirectory(storageDirectory);
LOG.info("Created BLOB server storage directory {}", storageDir);
// configure the maximum number of concurrent connections
- final int maxConnections = config.getInteger(
- ConfigConstants.BLOB_FETCH_CONCURRENT_KEY, ConfigConstants.DEFAULT_BLOB_FETCH_CONCURRENT);
+ final int maxConnections = config.getInteger(BlobServerOptions.FETCH_CONCURRENT);
if (maxConnections >= 1) {
--- End diff --
But I'm not sure if this should be part of this PR or a follow-up.
---
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 #3865: [FLINK-6518] Port blobserver config parameters to ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/3865
---
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 #3865: [FLINK-6518] Port blobserver config parameters to ...
Posted by greghogan <gi...@git.apache.org>.
Github user greghogan commented on a diff in the pull request:
https://github.com/apache/flink/pull/3865#discussion_r116222227
--- Diff: flink-core/src/main/java/org/apache/flink/configuration/BlobServerOptions.java ---
@@ -0,0 +1,76 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.flink.configuration;
+
+import org.apache.flink.annotation.PublicEvolving;
+
+import static org.apache.flink.configuration.ConfigOptions.key;
+
+/**
+ * Configuration options for the BlobServer.
+ */
+@PublicEvolving
--- End diff --
I expect we are doing this elsewhere but this deprecates refactored `@Public` APIs to `@PublicEvolving`. I understand the desire not to file a FLIP for this ticket, but should there be a pre-release evaluation of `@PublicEvolving` APIs for promotion to `@Public`?
---
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.
---