Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10829 )
Change subject: [DOCS] Clarification on admission control and DDL statements ...................................................................... Patch Set 2: (7 comments) Thanks for taking this on. I feel like there are still some remnants of some misconceptions here. Not sure exactly how best to fix it, but I feel like the misconceptions were so fundamental that we should just delete the existing text and figure out if there was a valid point to make. http://gerrit.cloudera.org:8080/#/c/10829/2/docs/topics/impala_admission.xml File docs/topics/impala_admission.xml: http://gerrit.cloudera.org:8080/#/c/10829/2/docs/topics/impala_admission.xml@820 PS2, Line 820: primarily queries, but also include statements that write data such as Maybe this could be clearer by stating that "Queries, DML statements and some DDL statements, including "CREATE TABLE AS SELECT" and "COMPUTE STATS" are affected by admission control. http://gerrit.cloudera.org:8080/#/c/10829/2/docs/topics/impala_admission.xml@822 PS2, Line 822: Most write operations in Impala are not resource-intensive, but This bit about "most write operations" feels very tangential, not sure how useful it is. If I was reading it I wouldn't know what it means. Does it mean that non-resource-intensive insert queries aren't affected by admission control (they are). Anyway, this isn't the exact problem we're trying to fix but noticed it while reading through. http://gerrit.cloudera.org:8080/#/c/10829/2/docs/topics/impala_admission.xml@847 PS2, Line 847: With or without admission control, all queries submitted in a session Unfortunately this isn't even true - it depends on the driver, the APIs give a lot of flexibility about launching multiple queries in a session at the same time and interacting with them in different ways while running. Different drivers can do different things here. I'm wondering if we could just omit this paragraph - admission control is orthogonal to sequencing of queries. All admission control can do is delay queries, which isn't really that different from them just running longer. http://gerrit.cloudera.org:8080/#/c/10829/2/docs/topics/impala_admission.xml@853 PS2, Line 853: Except for the <codeph>CREATE TABLE AS SELECT</codeph> statements, And compute stats http://gerrit.cloudera.org:8080/#/c/10829/2/docs/topics/impala_admission.xml@855 PS2, Line 855: for serial execution remove "for serial execution" - AC doesn't guarantee anything whether statements execute serially or concurrently. It can throttle statements but no reason 2, 10 or 100 statements can run concurrently. http://gerrit.cloudera.org:8080/#/c/10829/2/docs/topics/impala_admission.xml@855 PS2, Line 855: those DDL statements which does "those DDL statements" mean? It's a little unclear. I think it's actually wrong either way - maybe delete this sentence? http://gerrit.cloudera.org:8080/#/c/10829/2/docs/topics/impala_admission.xml@859 PS2, Line 859: <codeblock>-- This query is queued by admission control to avoid out-of-memory at times of heavy load. : SELECT * FROM huge_table JOIN enormous_table USING (id); : : -- This subsequent statement in the same session is also queued, but for serial execution and : -- not for admission control, until the previous statement completes. : DROP TABLE huge_table; : </codeblock> I think this example is still misleading. I'm not sure what it's demonstrating, because the behaviour is exactly the same with or without admission control. Is all of this trying to say "admission control is orthogonal to whether statements execute sequentially or not"? Maybe we should just find a better way to say that. E.g. "From the point of view of a client, queries that are queued by admission control behave the same as unqueued queries except that it takes longer to get results back." -- To view, visit http://gerrit.cloudera.org:8080/10829 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2e3e82bd34e88e7a13de1864aeb97f01023bc715 Gerrit-Change-Number: 10829 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni <[email protected]> Gerrit-Reviewer: Alex Rodoni <[email protected]> Gerrit-Reviewer: Balazs Jeszenszky <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Wed, 27 Jun 2018 01:05:18 +0000 Gerrit-HasComments: Yes
