Replacing CompletableFuture.supplyAsync() with fixed thread pool executor. CompletableFuture.supplyAsync() uses the common ForkJoinPool, which may not have enough threads.

[marinov-code]

…await() times out — which is not guaranteed.

For all changes:

• Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?

• Has your PR been rebased against the latest commit within the target branch (typically develop)?

• Is your initial contribution a single, squashed commit?

• Does gradlew build run cleanly?

• Have you written or updated unit tests to verify your changes?

• If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?

[raboof]

I think with this change the test would continue too quickly.

The goal of this line is to wait until the tasks have started. await().untilAsserted repeatedly runs () -> assertThat(service.getNumberOfCurrentExecutions()).isEqualTo(2) until it succeeds (or times out).

Changing the assertion to .isLessThanOrEqualTo(2) would make the test continue before both tasks have started, which I think is not what we want.

Do you see it time out? Perhaps we need to give it some more time before it gives up waiting for both tasks to start?

[marinov-code]

Hi @raboof, thank you for your review. I agree, it's better to keep the isEqualTo(2). I pushed a new fix replacing the CompletableFuture.supplyAsync() with fixed thread pool executor.

CompletableFuture.supplyAsync() uses the common ForkJoinPool, which may not have enough threads for all concurrent executions which will cause await().assertThat(service.getNumberOfCurrentExecutions()).isEqualTo(2) to timeout.

[semioticrobotic]

@CalvinKirs: Are you able to run the required workflows here? We're interested in determining whether the proposed patch fixes the tests.

[marinov-code]

Just a note, initially I couldn't reproduce the timeout issue locally. However after adding the following at geode-wan/build.gradle :

test {
  jvmArgs += [
          '-XX:ActiveProcessorCount=4'
  ]
}

I'm able to reproduce the issue every time I run the tests.

[CalvinKirs]

@CalvinKirs: Are you able to run the required workflows here? We're interested in determining whether the proposed patch fixes the tests.

done...thank you for your contribution

[marinov-code]

Hi @CalvinKirs, thank you for running the workflows but the build failed due to formatting issues. I just pushed a fix can you please run the workflows again?

[raboof]

It looks like with this change indeed the develop / unitTest tests all succeed, cool!

(acceptanceTest and integrationTest still fail, but that's likely a different issue)

[semioticrobotic]

(acceptanceTest and integrationTest still fail, but that's likely a different issue)

Yes, but @marinov-code is working diligently on this and has a fix in the works!