Skip to content

Conversation

@attilapiros
Copy link
Contributor

@attilapiros attilapiros commented Mar 3, 2021

What changes were proposed in this pull request?

Fixing the potentially flaky test in ExecutorPodsAllocatorSuite where multiple resource profiles are used:

  • SPARK-34361: scheduler backend known pods with multiple resource profiles at downscaling
  • SPARK-33288: multiple resource profiles

Why are the changes needed?

Resource profiles in executor PODs allocator are processed in a nondeterministic order.

Even as the tests are using Maps created by a constructor with two entries and the implementation behind is Map2 where the iterator keeps the ordering in the setTotalExpectedExecutors() method the entries are added to a ConcurrentHashMap totalExpectedExecutorsPerResourceProfileId.

So the resource allocation order might change from run to run.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Existing unit test.

@attilapiros
Copy link
Contributor Author

There is one more place to fix

@SparkQA
Copy link

SparkQA commented Mar 3, 2021

Test build #135703 has finished for PR 31719 at commit b291d22.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@attilapiros
Copy link
Contributor Author

attilapiros commented Mar 3, 2021

I wanted to be 100% sure that there is no more flakiness in the test. So temporary I have done a small code change and the sorted the map to test both asc and desc ordering. With this temporary code I have found a bug in an earlier test (in SPARK-33288: multiple resource profiles) which was there even before my SPARK-34361 PR.

So to be on the safe side I decided to modify the production code to have a deterministic ordering for allocating the resources( in case of multiple resource profiles).

@attilapiros attilapiros changed the title [SPARK-34361][K8S][FOLLOWUP] Fix flaky unit test SPARK-34361: scheduler backend known pods with multiple resource profiles at downscaling [SPARK-34361][K8S][FOLLOWUP] Fix potentially flaky unit test with multiple resource profiles in ExecutorPodsAllocatorSuite Mar 3, 2021
@SparkQA
Copy link

SparkQA commented Mar 3, 2021

Test build #135707 has finished for PR 31719 at commit 711ea68.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@attilapiros
Copy link
Contributor Author

closing this as flakiness is not reported by others

@attilapiros attilapiros closed this Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants