Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21803 )

Change subject: IMPALA-915: Support cancel queries during planning
......................................................................


Patch Set 13:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/21803/12/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
File fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java:

http://gerrit.cloudera.org:8080/#/c/21803/12/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java@305
PS12, Line 305:         // Ignore
> nit: we can remove all these "Ignore" comments
Removed from places where I added checkCancelled.


http://gerrit.cloudera.org:8080/#/c/21803/12/fe/src/main/java/org/apache/impala/catalog/ParallelFileMetadataLoader.java
File fe/src/main/java/org/apache/impala/catalog/ParallelFileMetadataLoader.java:

http://gerrit.cloudera.org:8080/#/c/21803/12/fe/src/main/java/org/apache/impala/catalog/ParallelFileMetadataLoader.java@195
PS12, Line 195:           Frontend.checkCancelled();
> Should we add a catalogd RPC for coordinator to cancel the metadata loading
I experimented with local-catalog mode and didn't really get the query thread 
to hang. I believe this all happens off the query thread.

I assumed the existing error handling here would cleanup an interrupted query, 
but I haven't tried to trace through it. I think cancelling a load in catalogd 
could be an interesting improvement. The only reason it might be necessary is 
if - by cancelling a query blocked on the table load - we could contribute to 
overloading catalogd with subsequent queries also trying to load tables.


http://gerrit.cloudera.org:8080/#/c/21803/12/tests/webserver/test_web_pages.py
File tests/webserver/test_web_pages.py:

http://gerrit.cloudera.org:8080/#/c/21803/12/tests/webserver/test_web_pages.py@1030
PS12, Line 1030: functional_parquet.alltypes
> This test runs concurrently with other e2e tests. We'd better use a unique
It's marked execute_serially, and needs to be to verify the relevant metrics.


http://gerrit.cloudera.org:8080/#/c/21803/12/tests/webserver/test_web_pages.py@1085
PS12, Line 1085:     delay_created_action = 
"impalad_load_tables_delay:SLEEP@5000"
> Can we add more tests setting catalogd_load_metadata_delay or catalogd_refr
I've tried these and see no evidence they're actually used.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d25d4c7fb0b8dcc7dad9510db1e8dca220eeb86
Gerrit-Change-Number: 21803
Gerrit-PatchSet: 13
Gerrit-Owner: Michael Smith <[email protected]>
Gerrit-Reviewer: Andrew Sherman <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Jason Fehr <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Comment-Date: Thu, 14 Nov 2024 00:16:38 +0000
Gerrit-HasComments: Yes

Reply via email to