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

Change subject: IMPALA-13772: Fix Workload Management DMLs Timeouts
......................................................................


Patch Set 6: Code-Review+1

(4 comments)

http://gerrit.cloudera.org:8080/#/c/22511/3/be/src/service/internal-server.cc
File be/src/service/internal-server.cc:

http://gerrit.cloudera.org:8080/#/c/22511/3/be/src/service/internal-server.cc@166
PS3, Line 166: n(debug_action->sec
> Good idea.  The issue is the tests need this debug action during the worklo
I see.. thanks for clarifying that.


http://gerrit.cloudera.org:8080/#/c/22511/6/be/src/service/workload-management-worker.cc
File be/src/service/workload-management-worker.cc:

http://gerrit.cloudera.org:8080/#/c/22511/6/be/src/service/workload-management-worker.cc@678
PS6, Line 678:   if (!FLAGS_debug_actions.empty()) {
             :     insert_query_opts[TImpalaQueryOptions::DEBUG_ACTION] = 
FLAGS_debug_actions;
             :   }
nit: Unrelated, but this makes me ask, should WM code follow 
--default_query_options when it does not have preference for some of the 
options? Or it has been handled by hs2 session somewhere?


http://gerrit.cloudera.org:8080/#/c/22511/3/tests/custom_cluster/test_admission_controller.py
File tests/custom_cluster/test_admission_controller.py:

http://gerrit.cloudera.org:8080/#/c/22511/3/tests/custom_cluster/test_admission_controller.py@45
PS3, Line 45:  tests.common.environ import build
> Yes, this function specifically sets up fair-scheduler.xml and llama-site.x
Done


http://gerrit.cloudera.org:8080/#/c/22511/3/tests/custom_cluster/test_query_log.py
File tests/custom_cluster/test_query_log.py:

http://gerrit.cloudera.org:8080/#/c/22511/3/tests/custom_cluster/test_query_log.py@1222
PS3, Line 1222:           "completed_queries", is_insert_query_queryid_success)
              :     assert retry(func=find_completed_insert_query, 
max_attempts=10, sleep_time_s=1,
              :                  backoff=1)
              :
              :
              : # Helper function to determine if a query from the debug UI is 
a workload management
              : # insert DML.
              : def _is_insert_query(query):
              :   return query["stmt"].lower().startswith("insert into 
{}".format(QUERY_TBL))
              :
              :
> That's a good question.  I've seen it both ways.  I prefer the global helpe
Ok, this is fine.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0cc7fbce40eadfb253d8cff5cbb83e2ad63a979f
Gerrit-Change-Number: 22511
Gerrit-PatchSet: 6
Gerrit-Owner: Jason Fehr <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Jason Fehr <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Comment-Date: Fri, 21 Feb 2025 18:10:46 +0000
Gerrit-HasComments: Yes

Reply via email to