Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21905 )

Change subject: IMPALA-11761: Run large dir tests serially
......................................................................


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/21905/1/tests/metadata/test_recursive_listing.py
File tests/metadata/test_recursive_listing.py:

http://gerrit.cloudera.org:8080/#/c/21905/1/tests/metadata/test_recursive_listing.py@171
PS1, Line 171: @pytest.mark.execute_serially
> Done. Although I'm skeptical the tests we have marked stress were actually
Agree. If there is no 'test_index', then it is not truly a stress test.


http://gerrit.cloudera.org:8080/#/c/21905/3/tests/pytest.ini
File tests/pytest.ini:

http://gerrit.cloudera.org:8080/#/c/21905/3/tests/pytest.ini@5
PS3, Line 5: run independent of other tests
nit: They do seems to execute separately from other test that not marked with 
'stress'. But if there are two or more test method is marked 'stress', all 
stress tests threads launch together and mix with one another, like below:
metadata/test_recursive_listing.py::TestRecursiveListing::test_partition_dir_removed_inflight[6]
metadata/test_recursive_listing.py::TestRecursiveListing::test_large_staging_dirs[6]
metadata/test_recursive_listing.py::TestRecursiveListing::test_large_staging_dirs[3]
metadata/test_recursive_listing.py::TestRecursiveListing::test_partition_dir_removed_inflight[0]
metadata/test_recursive_listing.py::TestRecursiveListing::test_large_staging_dirs[5]
metadata/test_recursive_listing.py::TestRecursiveListing::test_partition_dir_removed_inflight[5]
metadata/test_recursive_listing.py::TestRecursiveListing::test_partition_dir_removed_inflight[9]
metadata/test_recursive_listing.py::TestRecursiveListing::test_partition_dir_removed_inflight[7]
metadata/test_recursive_listing.py::TestRecursiveListing::test_large_staging_dirs[9]
metadata/test_recursive_listing.py::TestRecursiveListing::test_partition_dir_removed_inflight[2]
metadata/test_recursive_listing.py::TestRecursiveListing::test_large_staging_dirs[7]
metadata/test_recursive_listing.py::TestRecursiveListing::test_partition_dir_removed_inflight[4]
metadata/test_recursive_listing.py::TestRecursiveListing::test_large_staging_dirs[1]
metadata/test_recursive_listing.py::TestRecursiveListing::test_large_staging_dirs[2]
metadata/test_recursive_listing.py::TestRecursiveListing::test_partition_dir_removed_inflight[8]
metadata/test_recursive_listing.py::TestRecursiveListing::test_large_staging_dirs[8]
metadata/test_recursive_listing.py::TestRecursiveListing::test_large_staging_dirs[4]
metadata/test_recursive_listing.py::TestRecursiveListing::test_partition_dir_removed_inflight[3]
metadata/test_recursive_listing.py::TestRecursiveListing::test_partition_dir_removed_inflight[1]
metadata/test_recursive_listing.py::TestRecursiveListing::test_large_staging_dirs[0]


But marking both 'execute_serially' and 'stress' will additionally run all 
threads serially like below, just before that stress run:
metadata/test_recursive_listing.py::TestRecursiveListing::test_large_staging_dirs[0]
 PASSED                                                                         
                                  
metadata/test_recursive_listing.py::TestRecursiveListing::test_large_staging_dirs[1]
 PASSED                                                                         
                                  
metadata/test_recursive_listing.py::TestRecursiveListing::test_large_staging_dirs[2]
 PASSED                                                                         
                                  
metadata/test_recursive_listing.py::TestRecursiveListing::test_large_staging_dirs[3]
 PASSED                                                                         
                                  
metadata/test_recursive_listing.py::TestRecursiveListing::test_large_staging_dirs[4]
 PASSED                                                                         
                                  
metadata/test_recursive_listing.py::TestRecursiveListing::test_large_staging_dirs[5]
 PASSED                                                                         
                                  
metadata/test_recursive_listing.py::TestRecursiveListing::test_large_staging_dirs[6]
 PASSED                                                                         
                                  
metadata/test_recursive_listing.py::TestRecursiveListing::test_large_staging_dirs[7]
 PASSED                                                                         
                                  
metadata/test_recursive_listing.py::TestRecursiveListing::test_large_staging_dirs[8]
 PASSED                                                                         
                                  
metadata/test_recursive_listing.py::TestRecursiveListing::test_large_staging_dirs[9]
 PASSED                                                                         
                                  
metadata/test_recursive_listing.py::TestRecursiveListing::test_partition_dir_removed_inflight[0]
 PASSED                                                                         
                      
metadata/test_recursive_listing.py::TestRecursiveListing::test_partition_dir_removed_inflight[1]
 PASSED                                                                         
                      
metadata/test_recursive_listing.py::TestRecursiveListing::test_partition_dir_removed_inflight[2]
 PASSED                                                                         
                      
metadata/test_recursive_listing.py::TestRecursiveListing::test_partition_dir_removed_inflight[3]
 PASSED                                                                         
                      
metadata/test_recursive_listing.py::TestRecursiveListing::test_partition_dir_removed_inflight[4]
 PASSED                                                                         
                      
metadata/test_recursive_listing.py::TestRecursiveListing::test_partition_dir_removed_inflight[5]
 PASSED                                                                         
                      
metadata/test_recursive_listing.py::TestRecursiveListing::test_partition_dir_removed_inflight[6]
 PASSED                                                                         
                      
metadata/test_recursive_listing.py::TestRecursiveListing::test_partition_dir_removed_inflight[7]
 PASSED                                                                         
                      
metadata/test_recursive_listing.py::TestRecursiveListing::test_partition_dir_removed_inflight[8]
 PASSED                                                                         
                      
metadata/test_recursive_listing.py::TestRecursiveListing::test_partition_dir_removed_inflight[9]
 PASSED

Maybe the real intention of marking both 'execute_serially' and 'stress' is to 
have concurrent threads of same test method, but execute separately between 
different test method.

But in this case, I think just mark 'execute_serially' is OK.


http://gerrit.cloudera.org:8080/#/c/21905/3/tests/pytest.ini@5
PS3, Line 5: stress: stress tests to run at very high concurrency
Mention that test must have 'test_index' parameter.



--
To view, visit http://gerrit.cloudera.org:8080/21905
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1f7d2017ae1bab0f2f8cb0b100c2c6cc8b4f3dcd
Gerrit-Change-Number: 21905
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Comment-Date: Mon, 21 Oct 2024 18:50:17 +0000
Gerrit-HasComments: Yes

Reply via email to