You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2021/09/17 22:15:05 UTC
[GitHub] [geode] ringles opened a new pull request #6879: Revert "GEODE-9495: Limit Thread Sleep In PubSubNativeRedisAcceptance…
ringles opened a new pull request #6879:
URL: https://github.com/apache/geode/pull/6879
This reverts commit ed902f6404a1c2b06b980973a7a9d74f045fee8b. Not
working reliably in the pipeline.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] pivotal-jbarrett commented on a change in pull request #6879: Revert "GEODE-9495: Limit Thread Sleep In PubSubNativeRedisAcceptance…
Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on a change in pull request #6879:
URL: https://github.com/apache/geode/pull/6879#discussion_r711380082
##########
File path: geode-apis-compatible-with-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/executor/pubsub/PubSubNativeRedisAcceptanceTest.java
##########
@@ -31,52 +25,20 @@
public class PubSubNativeRedisAcceptanceTest extends AbstractPubSubIntegrationTest {
private static final Logger logger = LogService.getLogger();
- private static long socketTimeWaitMsec = 240000;
@ClassRule
public static NativeRedisTestRule redis = new NativeRedisTestRule();
- @BeforeClass
- public static void runOnce() throws IOException {
- if (SystemUtils.IS_OS_LINUX) {
- try {
- String line = getCommandOutput("cat", "/proc/sys/net/ipv4/tcp_fin_timeout");
- socketTimeWaitMsec = Long.parseLong(line.trim());
- } catch (NumberFormatException | IOException ignored) {
- }
- } else if (SystemUtils.IS_OS_MAC) {
- try {
- String line = getCommandOutput("sysctl", "net.inet.tcp.msl");
- String[] parts = line.split(":");
- if (parts.length == 2) {
- socketTimeWaitMsec = 2 * Long.parseLong(parts[1].trim());
- }
- } catch (NumberFormatException | IOException ignored) {
- }
- }
- // Just leave timeout at the default if it's some other OS or there's a problem getting OS value
- }
-
- private static String getCommandOutput(String... commandStringElements) throws IOException {
- Process process = new ProcessBuilder(commandStringElements).start();
- try (BufferedReader reader = new BufferedReader(
- new InputStreamReader(process.getInputStream()))) {
- return reader.readLine();
- } finally {
- // Probably overkill but ensures test leaves no orphaned processes
- process.destroy();
- }
- }
-
@AfterClass
public static void cleanup() throws InterruptedException {
// This test consumes a lot of sockets and any subsequent tests may fail because of spurious
// bind exceptions. Even though sockets are closed, they will remain in TIME_WAIT state so we
// need to wait for that to clear up. It shouldn't take more than a minute or so.
- // For now a thread sleep is the simplest way to wait for the sockets to be out of the TIME_WAIT
- // state. The default timeout of 240 sec was chosen because that is the default duration for
- // TIME_WAIT on Windows. The timeouts for both mac and linux are significantly shorter.
- Thread.sleep(socketTimeWaitMsec);
+ // There will be a better solution for this from GEODE-9495, but for now a thread sleep is the
+ // simplest way to wait for the sockets to be out of the TIME_WAIT state. The timeout of 240 sec
+ // was chosen because that is the default duration for TIME_WAIT on Windows. The timeouts for
+ // both mac and linux are significantly shorter.
+ Thread.sleep(240000);
Review comment:
How about configuring the host machines to cycle out of `TIME_WAT` faster and/or increasing the ephemeral port range?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] BenjaminPerryRoss merged pull request #6879: Revert "GEODE-9495: Limit Thread Sleep In PubSubNativeRedisAcceptance…
Posted by GitBox <gi...@apache.org>.
BenjaminPerryRoss merged pull request #6879:
URL: https://github.com/apache/geode/pull/6879
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] BenjaminPRoss commented on a change in pull request #6879: Revert "GEODE-9495: Limit Thread Sleep In PubSubNativeRedisAcceptance…
Posted by GitBox <gi...@apache.org>.
BenjaminPRoss commented on a change in pull request #6879:
URL: https://github.com/apache/geode/pull/6879#discussion_r711380710
##########
File path: geode-apis-compatible-with-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/executor/pubsub/PubSubNativeRedisAcceptanceTest.java
##########
@@ -31,52 +25,20 @@
public class PubSubNativeRedisAcceptanceTest extends AbstractPubSubIntegrationTest {
private static final Logger logger = LogService.getLogger();
- private static long socketTimeWaitMsec = 240000;
@ClassRule
public static NativeRedisTestRule redis = new NativeRedisTestRule();
- @BeforeClass
- public static void runOnce() throws IOException {
- if (SystemUtils.IS_OS_LINUX) {
- try {
- String line = getCommandOutput("cat", "/proc/sys/net/ipv4/tcp_fin_timeout");
- socketTimeWaitMsec = Long.parseLong(line.trim());
- } catch (NumberFormatException | IOException ignored) {
- }
- } else if (SystemUtils.IS_OS_MAC) {
- try {
- String line = getCommandOutput("sysctl", "net.inet.tcp.msl");
- String[] parts = line.split(":");
- if (parts.length == 2) {
- socketTimeWaitMsec = 2 * Long.parseLong(parts[1].trim());
- }
- } catch (NumberFormatException | IOException ignored) {
- }
- }
- // Just leave timeout at the default if it's some other OS or there's a problem getting OS value
- }
-
- private static String getCommandOutput(String... commandStringElements) throws IOException {
- Process process = new ProcessBuilder(commandStringElements).start();
- try (BufferedReader reader = new BufferedReader(
- new InputStreamReader(process.getInputStream()))) {
- return reader.readLine();
- } finally {
- // Probably overkill but ensures test leaves no orphaned processes
- process.destroy();
- }
- }
-
@AfterClass
public static void cleanup() throws InterruptedException {
// This test consumes a lot of sockets and any subsequent tests may fail because of spurious
// bind exceptions. Even though sockets are closed, they will remain in TIME_WAIT state so we
// need to wait for that to clear up. It shouldn't take more than a minute or so.
- // For now a thread sleep is the simplest way to wait for the sockets to be out of the TIME_WAIT
- // state. The default timeout of 240 sec was chosen because that is the default duration for
- // TIME_WAIT on Windows. The timeouts for both mac and linux are significantly shorter.
- Thread.sleep(socketTimeWaitMsec);
+ // There will be a better solution for this from GEODE-9495, but for now a thread sleep is the
+ // simplest way to wait for the sockets to be out of the TIME_WAIT state. The timeout of 240 sec
+ // was chosen because that is the default duration for TIME_WAIT on Windows. The timeouts for
+ // both mac and linux are significantly shorter.
+ Thread.sleep(240000);
Review comment:
I think the plan is to reintroduce the changes being reverted in this PR shortly, this commit has broken our acceptance tests so we're just removing it from develop to unblock the pipeline.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org