Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16549 )
Change subject: IMPALA-6671: Skip locked tables from topic updates ...................................................................... Patch Set 15: (7 comments) http://gerrit.cloudera.org:8080/#/c/16549/15/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java: http://gerrit.cloudera.org:8080/#/c/16549/15/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1311 PS15, Line 1311: to > nit: to get Done http://gerrit.cloudera.org:8080/#/c/16549/15/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1325 PS15, Line 1325: LOG.warn("Topic update thread blocking until lock is acquired for table {}", > This is too verbose when topicUpdateTblLockMaxWaitTimeMs_ is 0. Almost all makes sense. Done. http://gerrit.cloudera.org:8080/#/c/16549/15/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2235 PS15, Line 2235: the > nit: remove "the" Done http://gerrit.cloudera.org:8080/#/c/16549/15/tests/custom_cluster/test_topic_update_frequency.py File tests/custom_cluster/test_topic_update_frequency.py: http://gerrit.cloudera.org:8080/#/c/16549/15/tests/custom_cluster/test_topic_update_frequency.py@20 PS15, Line 20: @SkipIfS3.variable_listing_times > flake8: E302 expected 2 blank lines, found 1 Done http://gerrit.cloudera.org:8080/#/c/16549/15/tests/custom_cluster/test_topic_update_frequency.py@66 PS15, Line 66: is > nit: without? Thanks for pointing this out. http://gerrit.cloudera.org:8080/#/c/16549/15/tests/custom_cluster/test_topic_update_frequency.py@70 PS15, Line 70: is > nit: with? Done http://gerrit.cloudera.org:8080/#/c/16549/15/tests/custom_cluster/test_topic_update_frequency.py@106 PS15, Line 106: # there is no other good way other than to wait for some time to make sure that : # the slow query has been submitted and being compiled before we start the : # non_blocking queries : time.sleep(1) > Just for comments, we can use "wait_for_state" to wait for the CREATE state Actually, I had used this before in my earlier attempt but I found that to be flaky. My understanding is the CREATE state will be set when admission controller accepts the query? There is still a time delay between when the query is in create state and the query takes the table lock in catalogd. -- To view, visit http://gerrit.cloudera.org:8080/16549 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic657b96edbcdc94c6b906e7ca59291f4e4715655 Gerrit-Change-Number: 16549 Gerrit-PatchSet: 15 Gerrit-Owner: Vihang Karajgaonkar <vih...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Shant Hovsepian <sh...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Vihang Karajgaonkar <vih...@cloudera.com> Gerrit-Comment-Date: Mon, 21 Dec 2020 17:50:11 +0000 Gerrit-HasComments: Yes
