[GitHub] [flink] sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add connection and socket timeouts for the blob client
sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add
connection and socket timeouts for the blob client
URL: https://github.com/apache/flink/pull/8484#discussion_r288033314
##
File path:
flink-core/src/main/java/org/apache/flink/configuration/BlobServerOptions.java
##
@@ -102,4 +102,20 @@
public static final ConfigOption OFFLOAD_MINSIZE =
key("blob.offload.minsize")
.defaultValue(1_024 * 1_024) // 1MiB by default
.withDescription("The minimum size for messages to be offloaded
to the BlobServer.");
+
+ /**
+* The socket timeout in milliseconds for the blob client.
+*/
+ public static final ConfigOption SO_TIMEOUT =
+ key("blob.client.socket.timeout")
+ .defaultValue(120_000)
Review comment:
You're right. I didn't think of that. Or the default connection timeout is
set to `0`, while SO_TIMEOUT is still set to `2 minutes` ? @tillrohrmann
`SO_TIMEOUT` takes effect only after the connection is successful, so there
are no problems due to the limit of 50 concurrent connections. Setting the
connection timeout to `0` will not break existing setups, and setting the
socket timeout to the value greater than 0 can fix the deadlock reported by
this issue.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add connection and socket timeouts for the blob client
sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add
connection and socket timeouts for the blob client
URL: https://github.com/apache/flink/pull/8484#discussion_r288033314
##
File path:
flink-core/src/main/java/org/apache/flink/configuration/BlobServerOptions.java
##
@@ -102,4 +102,20 @@
public static final ConfigOption OFFLOAD_MINSIZE =
key("blob.offload.minsize")
.defaultValue(1_024 * 1_024) // 1MiB by default
.withDescription("The minimum size for messages to be offloaded
to the BlobServer.");
+
+ /**
+* The socket timeout in milliseconds for the blob client.
+*/
+ public static final ConfigOption SO_TIMEOUT =
+ key("blob.client.socket.timeout")
+ .defaultValue(120_000)
Review comment:
You're right. I didn't think of that. Or the default connection timeout is
set to `0`, while SO_TIMEOUT is still set to `2 minutes` ?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add connection and socket timeouts for the blob client
sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add
connection and socket timeouts for the blob client
URL: https://github.com/apache/flink/pull/8484#discussion_r287917746
##
File path:
flink-core/src/main/java/org/apache/flink/configuration/BlobServerOptions.java
##
@@ -102,4 +102,20 @@
public static final ConfigOption OFFLOAD_MINSIZE =
key("blob.offload.minsize")
.defaultValue(1_024 * 1_024) // 1MiB by default
.withDescription("The minimum size for messages to be offloaded
to the BlobServer.");
+
+ /**
+* The socket timeout in milliseconds for the blob client.
+*/
+ public static final ConfigOption SO_TIMEOUT =
+ key("blob.client.socket.timeout")
+ .defaultValue(120_000)
+ .withDescription("The socket timeout in milliseconds
for the blob client.");
+
+ /**
+* The connection timeout in milliseconds for the blob client.
+*/
+ public static final ConfigOption CONNECT_TIMEOUT =
+ key("blob.client.connect.timeout")
+ .defaultValue(120_000)
Review comment:
Please look at my above comments.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add connection and socket timeouts for the blob client
sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add
connection and socket timeouts for the blob client
URL: https://github.com/apache/flink/pull/8484#discussion_r287915194
##
File path:
flink-core/src/main/java/org/apache/flink/configuration/BlobServerOptions.java
##
@@ -102,4 +102,20 @@
public static final ConfigOption OFFLOAD_MINSIZE =
key("blob.offload.minsize")
.defaultValue(1_024 * 1_024) // 1MiB by default
.withDescription("The minimum size for messages to be offloaded
to the BlobServer.");
+
+ /**
+* The socket timeout in milliseconds for the blob client.
+*/
+ public static final ConfigOption SO_TIMEOUT =
+ key("blob.client.socket.timeout")
+ .defaultValue(120_000)
Review comment:
Check the option `taskmanager.network.netty.client.connectTimeoutSec`, whose
default value is also `120s`. Let we keep connection/socket timeouts consistent
with it? @tillrohrmann
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add connection and socket timeouts for the blob client
sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add
connection and socket timeouts for the blob client
URL: https://github.com/apache/flink/pull/8484#discussion_r287915194
##
File path:
flink-core/src/main/java/org/apache/flink/configuration/BlobServerOptions.java
##
@@ -102,4 +102,20 @@
public static final ConfigOption OFFLOAD_MINSIZE =
key("blob.offload.minsize")
.defaultValue(1_024 * 1_024) // 1MiB by default
.withDescription("The minimum size for messages to be offloaded
to the BlobServer.");
+
+ /**
+* The socket timeout in milliseconds for the blob client.
+*/
+ public static final ConfigOption SO_TIMEOUT =
+ key("blob.client.socket.timeout")
+ .defaultValue(120_000)
Review comment:
Check the option `taskmanager.network.netty.client.connectTimeoutSec`, whose
default value is set to `120s`. Let we keep connection/socket timeouts
consistent with it? @tillrohrmann
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add connection and socket timeouts for the blob client
sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add
connection and socket timeouts for the blob client
URL: https://github.com/apache/flink/pull/8484#discussion_r287915194
##
File path:
flink-core/src/main/java/org/apache/flink/configuration/BlobServerOptions.java
##
@@ -102,4 +102,20 @@
public static final ConfigOption OFFLOAD_MINSIZE =
key("blob.offload.minsize")
.defaultValue(1_024 * 1_024) // 1MiB by default
.withDescription("The minimum size for messages to be offloaded
to the BlobServer.");
+
+ /**
+* The socket timeout in milliseconds for the blob client.
+*/
+ public static final ConfigOption SO_TIMEOUT =
+ key("blob.client.socket.timeout")
+ .defaultValue(120_000)
Review comment:
Check the option `taskmanager.network.netty.client.connectTimeoutSec`, whose
default value is set to `120s`, and let we keep connection/socket timeouts
consistent with it. What do you think? @tillrohrmann
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add connection and socket timeouts for the blob client
sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add
connection and socket timeouts for the blob client
URL: https://github.com/apache/flink/pull/8484#discussion_r287633012
##
File path:
flink-core/src/main/java/org/apache/flink/configuration/BlobServerOptions.java
##
@@ -102,4 +102,20 @@
public static final ConfigOption OFFLOAD_MINSIZE =
key("blob.offload.minsize")
.defaultValue(1_024 * 1_024) // 1MiB by default
.withDescription("The minimum size for messages to be offloaded
to the BlobServer.");
+
+ /**
+* The socket timeout in milliseconds for the blob client.
+*/
+ public static final ConfigOption SO_TIMEOUT =
+ key("blob.client.socket.timeout")
+ .defaultValue(120_000)
Review comment:
` 2 minutes` is a value that I give freely, and it has little basis, just
considering that a slightly longer wait may be required in the case of high CPU
load. In fact, the blob client has a retry mechanism, and we should set it a
smaller. Do you have any recommended values about connection and socket
timeouts? (The default value of connection timeout is 60s in Netty.)
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add connection and socket timeouts for the blob client
sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add
connection and socket timeouts for the blob client
URL: https://github.com/apache/flink/pull/8484#discussion_r287633012
##
File path:
flink-core/src/main/java/org/apache/flink/configuration/BlobServerOptions.java
##
@@ -102,4 +102,20 @@
public static final ConfigOption OFFLOAD_MINSIZE =
key("blob.offload.minsize")
.defaultValue(1_024 * 1_024) // 1MiB by default
.withDescription("The minimum size for messages to be offloaded
to the BlobServer.");
+
+ /**
+* The socket timeout in milliseconds for the blob client.
+*/
+ public static final ConfigOption SO_TIMEOUT =
+ key("blob.client.socket.timeout")
+ .defaultValue(120_000)
Review comment:
` 2 minutes` is a value that I give freely, and it has little basis, just
considering that a slightly longer wait may be required in the case of high CPU
load. In fact, the blob client has a retry mechanism, and we should set it a
smaller. Do you have any recommended values about connection and socket
timeouts?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add connection and socket timeouts for the blob client
sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add
connection and socket timeouts for the blob client
URL: https://github.com/apache/flink/pull/8484#discussion_r287633012
##
File path:
flink-core/src/main/java/org/apache/flink/configuration/BlobServerOptions.java
##
@@ -102,4 +102,20 @@
public static final ConfigOption OFFLOAD_MINSIZE =
key("blob.offload.minsize")
.defaultValue(1_024 * 1_024) // 1MiB by default
.withDescription("The minimum size for messages to be offloaded
to the BlobServer.");
+
+ /**
+* The socket timeout in milliseconds for the blob client.
+*/
+ public static final ConfigOption SO_TIMEOUT =
+ key("blob.client.socket.timeout")
+ .defaultValue(120_000)
Review comment:
` 2 minutes` is a value that I give freely, and it has little basis, just
considering that a slightly longer wait may be required in the case of high CPU
load. In fact, the blob client has a retry mechanism, and we should set it a
little smaller. Do you have any recommended values about connection and socket
timeouts?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add connection and socket timeouts for the blob client
sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add
connection and socket timeouts for the blob client
URL: https://github.com/apache/flink/pull/8484#discussion_r287638025
##
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/blob/BlobClientSslTest.java
##
@@ -70,7 +70,7 @@ public static void startNonSSLServer() throws IOException {
config.setString(BlobServerOptions.STORAGE_DIRECTORY,
temporarySslFolder.newFolder().getAbsolutePath());
config.setBoolean(BlobServerOptions.SSL_ENABLED, false);
- blobNonSslServer = new BlobServer(config, new VoidBlobStore());
+ blobNonSslServer = new TestBlobServer(config, new
VoidBlobStore());
Review comment:
I checked the code carefully, and `TestBlobServer` is not needed here. I
will revert to `BlobServer`.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add connection and socket timeouts for the blob client
sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add
connection and socket timeouts for the blob client
URL: https://github.com/apache/flink/pull/8484#discussion_r287637335
##
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/blob/BlobClientSslTest.java
##
@@ -58,7 +58,7 @@ public static void startSSLServer() throws IOException {
Configuration config =
SSLUtilsTest.createInternalSslConfigWithKeyAndTrustStores();
config.setString(BlobServerOptions.STORAGE_DIRECTORY,
temporarySslFolder.newFolder().getAbsolutePath());
- blobSslServer = new BlobServer(config, new VoidBlobStore());
+ blobSslServer = new TestBlobServer(config, new VoidBlobStore());
Review comment:
`BlobClientSslTest` inherits `BlobClientTest`, and it will also execute the
new test `testSocketTimeout` added in `BlobClientTest`. If `TestBlobServer` is
not used here, an error will occur.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add connection and socket timeouts for the blob client
sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add
connection and socket timeouts for the blob client
URL: https://github.com/apache/flink/pull/8484#discussion_r287635535
##
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/blob/BlobClientSslTest.java
##
@@ -70,7 +70,7 @@ public static void startNonSSLServer() throws IOException {
config.setString(BlobServerOptions.STORAGE_DIRECTORY,
temporarySslFolder.newFolder().getAbsolutePath());
config.setBoolean(BlobServerOptions.SSL_ENABLED, false);
- blobNonSslServer = new BlobServer(config, new VoidBlobStore());
+ blobNonSslServer = new TestBlobServer(config, new
VoidBlobStore());
Review comment:
`BlobClientSslTest` inherits `BlobClientTest`, and it will also execute the
new test `testSocketTimeout` added in `BlobClientTest`. If `TestBlobServer` is
not used here, an error will occur.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add connection and socket timeouts for the blob client
sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add
connection and socket timeouts for the blob client
URL: https://github.com/apache/flink/pull/8484#discussion_r287635709
##
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/blob/BlobClientTest.java
##
@@ -487,4 +488,58 @@ private static void uploadJarFile(
validateGetAndClose(blobClient.getInternal(jobId,
blobKeys.get(0)), testFile);
}
}
+
+
+ /**
+* Tests the socket operation timeout.
+*/
+ @Test
+ public void testSocketTimeout() {
+ Configuration clientConfig = getBlobClientConfig();
+ int oldSoTimeout =
clientConfig.getInteger(BlobServerOptions.SO_TIMEOUT);
+
+ clientConfig.setInteger(BlobServerOptions.SO_TIMEOUT, 50);
+ getBlobServer().setBlockingMillis(10_000);
+
+ try {
+ InetSocketAddress serverAddress = new
InetSocketAddress("localhost", getBlobServer().getPort());
+
+ try (BlobClient client = new BlobClient(serverAddress,
clientConfig)) {
+ client.getInternal(new JobID(),
BlobKey.createKey(TRANSIENT_BLOB));
+
+ fail("Should throw an exception.");
+ } catch (Throwable t) {
+
assertEquals(java.net.SocketTimeoutException.class,
ExceptionUtils.stripException(t, IOException.class).getClass());
Review comment:
> We could create a new test class which contains all blob server tests
which need to start a new blob server for each test.
It's a good idea. But considering that only one test currently has this
need, in order to reduce the maintenance cost of the code, I think we can
create a new test class when more tests have this need in the future.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add connection and socket timeouts for the blob client
sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add
connection and socket timeouts for the blob client
URL: https://github.com/apache/flink/pull/8484#discussion_r287635709
##
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/blob/BlobClientTest.java
##
@@ -487,4 +488,58 @@ private static void uploadJarFile(
validateGetAndClose(blobClient.getInternal(jobId,
blobKeys.get(0)), testFile);
}
}
+
+
+ /**
+* Tests the socket operation timeout.
+*/
+ @Test
+ public void testSocketTimeout() {
+ Configuration clientConfig = getBlobClientConfig();
+ int oldSoTimeout =
clientConfig.getInteger(BlobServerOptions.SO_TIMEOUT);
+
+ clientConfig.setInteger(BlobServerOptions.SO_TIMEOUT, 50);
+ getBlobServer().setBlockingMillis(10_000);
+
+ try {
+ InetSocketAddress serverAddress = new
InetSocketAddress("localhost", getBlobServer().getPort());
+
+ try (BlobClient client = new BlobClient(serverAddress,
clientConfig)) {
+ client.getInternal(new JobID(),
BlobKey.createKey(TRANSIENT_BLOB));
+
+ fail("Should throw an exception.");
+ } catch (Throwable t) {
+
assertEquals(java.net.SocketTimeoutException.class,
ExceptionUtils.stripException(t, IOException.class).getClass());
Review comment:
> We could create a new test class which contains all blob server tests
which need to start a new blob server for each test.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add connection and socket timeouts for the blob client
sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add
connection and socket timeouts for the blob client
URL: https://github.com/apache/flink/pull/8484#discussion_r287635535
##
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/blob/BlobClientSslTest.java
##
@@ -70,7 +70,7 @@ public static void startNonSSLServer() throws IOException {
config.setString(BlobServerOptions.STORAGE_DIRECTORY,
temporarySslFolder.newFolder().getAbsolutePath());
config.setBoolean(BlobServerOptions.SSL_ENABLED, false);
- blobNonSslServer = new BlobServer(config, new VoidBlobStore());
+ blobNonSslServer = new TestBlobServer(config, new
VoidBlobStore());
Review comment:
`BlobClientSslTest` inherits `BlobClientTest`, and it will also execute the
new test `testSocketTimeout` added in `BlobClientTest`. If `TestBlobServer` is
not used here, an error will occur.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add connection and socket timeouts for the blob client
sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add
connection and socket timeouts for the blob client
URL: https://github.com/apache/flink/pull/8484#discussion_r287633012
##
File path:
flink-core/src/main/java/org/apache/flink/configuration/BlobServerOptions.java
##
@@ -102,4 +102,20 @@
public static final ConfigOption OFFLOAD_MINSIZE =
key("blob.offload.minsize")
.defaultValue(1_024 * 1_024) // 1MiB by default
.withDescription("The minimum size for messages to be offloaded
to the BlobServer.");
+
+ /**
+* The socket timeout in milliseconds for the blob client.
+*/
+ public static final ConfigOption SO_TIMEOUT =
+ key("blob.client.socket.timeout")
+ .defaultValue(120_000)
Review comment:
` 2 minutes` is a value that I give freely, and it has little basis, just
considering that a slightly longer wait may be required in the case of high CPU
load. In fact, the blob client has a retry mechanism. and we should set it a
little smaller. Do you have any recommended values about connection and socket
timeouts?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add connection and socket timeouts for the blob client
sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add
connection and socket timeouts for the blob client
URL: https://github.com/apache/flink/pull/8484#discussion_r287197990
##
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/blob/BlobClientTest.java
##
@@ -487,4 +488,58 @@ private static void uploadJarFile(
validateGetAndClose(blobClient.getInternal(jobId,
blobKeys.get(0)), testFile);
}
}
+
+
+ /**
+* Tests the socket operation timeout.
+*/
+ @Test
+ public void testSocketTimeout() {
+ Configuration clientConfig = getBlobClientConfig();
+ int oldSoTimeout =
clientConfig.getInteger(BlobServerOptions.SO_TIMEOUT);
+
+ clientConfig.setInteger(BlobServerOptions.SO_TIMEOUT, 50);
+ getBlobServer().setBlockingMillis(10_000);
+
+ try {
+ InetSocketAddress serverAddress = new
InetSocketAddress("localhost", getBlobServer().getPort());
+
+ try (BlobClient client = new BlobClient(serverAddress,
clientConfig)) {
+ client.getInternal(new JobID(),
BlobKey.createKey(TRANSIENT_BLOB));
+
+ fail("Should throw a exception.");
+ } catch (Throwable t) {
+
assertEquals(java.net.SocketTimeoutException.class,
ExceptionUtils.stripException(t, IOException.class).getClass());
+ }
+ } finally {
+ clientConfig.setInteger(BlobServerOptions.SO_TIMEOUT,
oldSoTimeout);
Review comment:
In that case, the blob server need to be started every time, which will
lengthen the test time.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink] sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add connection and socket timeouts for the blob client
sunhaibotb commented on a change in pull request #8484: [FLINK-12547] Add
connection and socket timeouts for the blob client
URL: https://github.com/apache/flink/pull/8484#discussion_r287197202
##
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/blob/BlobClientTest.java
##
@@ -487,4 +488,58 @@ private static void uploadJarFile(
validateGetAndClose(blobClient.getInternal(jobId,
blobKeys.get(0)), testFile);
}
}
+
+
+ /**
+* Tests the socket operation timeout.
+*/
+ @Test
+ public void testSocketTimeout() {
+ Configuration clientConfig = getBlobClientConfig();
+ int oldSoTimeout =
clientConfig.getInteger(BlobServerOptions.SO_TIMEOUT);
+
+ clientConfig.setInteger(BlobServerOptions.SO_TIMEOUT, 50);
+ getBlobServer().setBlockingMillis(10_000);
+
+ try {
+ InetSocketAddress serverAddress = new
InetSocketAddress("localhost", getBlobServer().getPort());
+
+ try (BlobClient client = new BlobClient(serverAddress,
clientConfig)) {
+ client.getInternal(new JobID(),
BlobKey.createKey(TRANSIENT_BLOB));
+
+ fail("Should throw a exception.");
+ } catch (Throwable t) {
+
assertEquals(java.net.SocketTimeoutException.class,
ExceptionUtils.stripException(t, IOException.class).getClass());
Review comment:
I think asserting the exception type is sufficient.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
