Skip to content

[Tests] Fix flaky tests and memory leak related to ContainerAllocator tests#1556

Merged
cameronlee314 merged 1 commit intoapache:masterfrom
cameronlee314:containerallocflaky
Nov 10, 2021
Merged

[Tests] Fix flaky tests and memory leak related to ContainerAllocator tests#1556
cameronlee314 merged 1 commit intoapache:masterfrom
cameronlee314:containerallocflaky

Conversation

@cameronlee314
Copy link
Contributor

@cameronlee314 cameronlee314 commented Nov 9, 2021

Issues:

  1. TestContainerAllocatorWithHostAffinity.testExpiredRequestsAreCancelled fails transiently due to seeing a value of 4 for cancelledRequests.size() instead of 3.
  2. samza-core tests take a lot of heap to run (3GB), but it seems like they shouldn't need that much.

Changes:

  1. Change the cluster-manager.container.request.timeout.ms to be 500ms instead of 3ms in TestContainerAllocatorWithHostAffinity in order to prevent the test from cancelling requests too quickly and causing an extra cancellation to happen before the validation is done.
  2. Stop the ContainerAllocator and ContainerPlacementManager objects in tests so that their corresponding threads can exit. Before, I believe the threads just kept running, and that may have eaten up memory.
  3. Reduce the heap needed for samza-core tests.

Tests: Ran CI build multiple times

API changes: N/A

@cameronlee314 cameronlee314 changed the title [DO NOT REVIEW] [WIP] flaky ContainerAllocator tests and potential memory leak [Tests] Fix flaky tests and memory leak related to ContainerAllocator tests Nov 9, 2021
Copy link
Contributor

@Sanil15 Sanil15 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank for the Fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants