Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

LocalRunnerTest failing in CI #11619

Closed
bikramSingh91 opened this issue Nov 21, 2024 · 0 comments
Closed

LocalRunnerTest failing in CI #11619

bikramSingh91 opened this issue Nov 21, 2024 · 0 comments
Assignees
@kagamiori
Labels
broken-test A broken or flaky test that fails CI or pre-commit tests bug Something isn't working

Comments

@bikramSingh91
Copy link
Contributor

Bug description

Failed on one of the test build on my PR but it looks like its also failing on main branch

System information

machines used by github actions CI

Relevant logs

387/394 Test #392: velox_local_runner_test ..............................................***Failed    3.27 sec
[==========] Running 3 tests from 1 test suite.
[----------] Global test environment set-up.
[----------] 3 tests from LocalRunnerTest
[ RUN      ] LocalRunnerTest.count
I1121 20:03:38.006389 81230 Compression.cpp:627] Initialized zstd compressor with compression level 7
[       OK ] LocalRunnerTest.count (1106 ms)
[ RUN      ] LocalRunnerTest.error
E20241121 20:03:39.149382 81230 Exceptions.h:66] Line: /__w/velox/velox/velox/runner/LocalRunner.cpp:124, Function:waitForCompletion, Expression:  LocalRunner did not finish within 5000 us, Source: RUNTIME, ErrorCode: INVALID_STATE
unknown file: Failure
C++ exception with description "Exception: VeloxRuntimeError
Error Source: RUNTIME
Error Code: INVALID_STATE
Reason: LocalRunner did not finish within 5000 us
Retriable: False
Function: waitForCompletion
File: /__w/velox/velox/velox/runner/LocalRunner.cpp
Line: 124
Stack trace:
# 0  _ZN8facebook5velox7process10StackTraceC1Ei
# 1  _ZN8facebook5velox14VeloxExceptionC1EPKcmS3_St17basic_string_viewIcSt11char_traitsIcEES7_S7_S7_bNS1_4TypeES7_
# 2  _ZN8facebook5velox6detail14veloxCheckFailINS0_17VeloxRuntimeErrorERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEEEvRKNS1_18VeloxCheckFailArgsET0_
# 3  _ZN8facebook5velox6runner11LocalRunner17waitForCompletionEi
# 4  _ZN26LocalRunnerTest_error_Test8TestBodyEv
# 5  _ZN7testing8internal35HandleExceptionsInMethodIfSupportedINS_4TestEvEET0_PT_MS4_FS3_vEPKc
# 6  _ZN7testing4Test3RunEv
# 7  _ZN7testing8TestInfo3RunEv
# 8  _ZN7testing9TestSuite3RunEv
# 9  _ZN7testing8internal12UnitTestImpl11RunAllTestsEv
# 10 _ZN7testing8UnitTest3RunEv
# 11 main
# 12 __libc_start_call_main
# 13 __libc_start_main
# 14 _start
" thrown in the test body.
[  FAILED  ] LocalRunnerTest.error (1035 ms)
[ RUN      ] LocalRunnerTest.scan
[       OK ] LocalRunnerTest.scan (1019 ms)
[----------] 3 tests from LocalRunnerTest (3162 ms total)

[----------] Global test environment tear-down
[==========] 3 tests from 1 test suite ran. (3199 ms total)
[  PASSED  ] 2 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] LocalRunnerTest.error
@bikramSingh91 bikramSingh91 added bug Something isn't working triage Newly created issue that needs attention. labels Nov 21, 2024
@bikramSingh91 bikramSingh91 mentioned this issue Nov 21, 2024
Closed
@kagamiori kagamiori assigned kagamiori and unassigned oerling Nov 21, 2024
@bikramSingh91 bikramSingh91 added broken-test A broken or flaky test that fails CI or pre-commit tests and removed triage Newly created issue that needs attention. labels Nov 21, 2024
@kagamiori kagamiori mentioned this issue Nov 25, 2024
Closed
This was referenced Nov 25, 2024
Closed
Closed
@jinchengchenghh jinchengchenghh mentioned this issue Nov 27, 2024
Closed
Yuhta added a commit to Yuhta/velox that referenced this issue Nov 29, 2024
@Yuhta @facebook-github-bot
fix: Buckify local runner and fix flaky LocalRunnerTest
d9b7fd2 
Summary: Fix facebookincubator#11619

Differential Revision: D66600801
This was referenced Nov 29, 2024
Closed
Closed
@jinchengchenghh jinchengchenghh mentioned this issue Dec 2, 2024
Closed
@kagamiori kagamiori mentioned this issue Dec 2, 2024
Closed
facebook-github-bot pushed a commit that referenced this issue Dec 3, 2024
@kagamiori @facebook-github-bot
fix: Fix flaky velox_local_runner_test due to memory pool name collis…
a969af8 
…ion (#11683)

Summary:
velox_local_runner_test was flaky because all test cases use the same root
pool and the same query id. As the result, they attempt to add leaf pools
with the same name into the root pool. However, because the maxWaitUs
sent to LocalRunner::waitForCompletion() is small, sometimes a test case
starts creating a task before the task of the last test case has been destructed.
This causes the root pool seeing a request of adding a child pool with a name
already exists, hence causing the test failure. This diff updates the unit tests to
create a separate root pool for every query.

This diff fixes #11619.

Pull Request resolved: #11683

Reviewed By: xiaoxmeng

Differential Revision: D66678119

Pulled By: kagamiori

fbshipit-source-id: 3d02793d9da406959ddf86d0ef666e9038da7434
TongWei1105 pushed a commit to TongWei1105/velox that referenced this issue Dec 3, 2024
fix: Buckify local runner and fix flaky LocalRunnerTest (facebookincu…
d13fae5 
…bator#11694)

Summary:
Pull Request resolved: facebookincubator#11694

Fix facebookincubator#11619

Reviewed By: amitkdutta

Differential Revision: D66600801

fbshipit-source-id: a9e8d7633fedfcf35d9919c73bbc5dc9437be6af
TongWei1105 pushed a commit to TongWei1105/velox that referenced this issue Dec 3, 2024
fix: Fix flaky velox_local_runner_test due to memory pool name collis…
3edbdfa 
…ion (facebookincubator#11683)

Summary:
velox_local_runner_test was flaky because all test cases use the same root
pool and the same query id. As the result, they attempt to add leaf pools
with the same name into the root pool. However, because the maxWaitUs
sent to LocalRunner::waitForCompletion() is small, sometimes a test case
starts creating a task before the task of the last test case has been destructed.
This causes the root pool seeing a request of adding a child pool with a name
already exists, hence causing the test failure. This diff updates the unit tests to
create a separate root pool for every query.

This diff fixes facebookincubator#11619.

Pull Request resolved: facebookincubator#11683

Reviewed By: xiaoxmeng

Differential Revision: D66678119

Pulled By: kagamiori

fbshipit-source-id: 3d02793d9da406959ddf86d0ef666e9038da7434
athmaja-n pushed a commit to athmaja-n/velox that referenced this issue Jan 10, 2025
@Yuhta @athmaja-n
fix: Buckify local runner and fix flaky LocalRunnerTest (facebookincu…
412b5b4 
…bator#11694)

Summary:
Pull Request resolved: facebookincubator#11694

Fix facebookincubator#11619

Reviewed By: amitkdutta

Differential Revision: D66600801

fbshipit-source-id: a9e8d7633fedfcf35d9919c73bbc5dc9437be6af
athmaja-n pushed a commit to athmaja-n/velox that referenced this issue Jan 10, 2025
@kagamiori @athmaja-n
fix: Fix flaky velox_local_runner_test due to memory pool name collis…
a7a2d76 
…ion (facebookincubator#11683)

Summary:
velox_local_runner_test was flaky because all test cases use the same root
pool and the same query id. As the result, they attempt to add leaf pools
with the same name into the root pool. However, because the maxWaitUs
sent to LocalRunner::waitForCompletion() is small, sometimes a test case
starts creating a task before the task of the last test case has been destructed.
This causes the root pool seeing a request of adding a child pool with a name
already exists, hence causing the test failure. This diff updates the unit tests to
create a separate root pool for every query.

This diff fixes facebookincubator#11619.

Pull Request resolved: facebookincubator#11683

Reviewed By: xiaoxmeng

Differential Revision: D66678119

Pulled By: kagamiori

fbshipit-source-id: 3d02793d9da406959ddf86d0ef666e9038da7434
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kagamiori kagamiori

Labels
broken-test A broken or flaky test that fails CI or pre-commit tests bug Something isn't working
Projects
None yet
Milestone
No milestone
3 participants
@bikramSingh91 @kagamiori @oerling