Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/22231 )

Change subject: IMPALA-13620: Refresh compute_table_stats.py script
......................................................................


Patch Set 10:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/22231/10/tests/util/compute_table_stats.py
File tests/util/compute_table_stats.py:

http://gerrit.cloudera.org:8080/#/c/22231/10/tests/util/compute_table_stats.py@104
PS10, Line 104:         if continue_on_error or len(exceptions) == 0:
apply_async() is instantaneous, so there's no time for an exception to get back 
to us. I think this will submit all tasks and then sit in pool.join() waiting 
for all of them.


http://gerrit.cloudera.org:8080/#/c/22231/10/tests/util/compute_table_stats.py@112
PS10, Line 112:     LOG.info("Completed COMPUTE STATS for {}/{} tables ".format(
              :       len(success_tables), total_tables))
If continue_on_error is true, the exception is not being reraised. There is no 
return statement on that path, but the function is implicitly returning None 
and will run append_success_table() with None. So, the number of successful 
tables won't be right for that case. I think the number of successful tables 
would be right for the continue_on_error=False case.

Here's one option:
1. Start with the original code
2. Add in the excluded tables logic
3. Change compute_stats_table() so it always rethrows the exception (i.e. it 
does not need continue_on_error anymore).
4. Catch exceptions from the f.get() in the original code. If 
continue_on_error=True, use it for accounting the number of successes / 
failures. If continue_on_error=False, bail out on the first failure. (I assume 
that's what we want).



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ebf02f95b957e7dda3a30622b87e8fca3197699
Gerrit-Change-Number: 22231
Gerrit-PatchSet: 10
Gerrit-Owner: Riza Suminto <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Jason Fehr <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Comment-Date: Wed, 08 Jan 2025 00:15:50 +0000
Gerrit-HasComments: Yes

Reply via email to