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

Change subject: IMPALA-13186: Tag query option scope for tuple cache
......................................................................


Patch Set 9:

(5 comments)

Thinking about which query options should be exempt, I'm trying to think about 
which ones could vary from session to session or query to query. That could 
happen if sessions map to a different resource pool (which can specify default 
query options for that resource pool). That makes me think that we should 
exempt mem_limit.

I'm looking through the list to see if others stand out.

The general risk is that some customer could have lots of query option 
variability and have bad cache hit rate.

http://gerrit.cloudera.org:8080/#/c/21698/9/be/src/service/query-options.h
File be/src/service/query-options.h:

http://gerrit.cloudera.org:8080/#/c/21698/9/be/src/service/query-options.h@77
PS9, Line 77:   QUERY_OPT_FN(mem_limit, MEM_LIMIT, TQueryOptionLevel::REGULAR)  
                       \
I could see this one being different across different queries or changing over 
time. I think we want to share results across queries with different values for 
this.

I think we should make this exempt.


http://gerrit.cloudera.org:8080/#/c/21698/9/be/src/service/query-options.h@83
PS9, Line 83:   QUERY_OPT_FN(request_pool, REQUEST_POOL, 
TQueryOptionLevel::REGULAR)                   \
I think we want to share results across different request pools. I think we 
should make this exempt.


http://gerrit.cloudera.org:8080/#/c/21698/9/be/src/service/query-options.h@194
PS9, Line 194:   QUERY_OPT_FN(statement_expression_limit, 
STATEMENT_EXPRESSION_LIMIT,                   \
             :       TQueryOptionLevel::REGULAR)                                
                        \
Let's make this exempt. It applies a limit, but it otherwise has no impact on 
execution.


http://gerrit.cloudera.org:8080/#/c/21698/9/be/src/service/query-options.h@211
PS9, Line 211:   QUERY_OPT_FN(mem_limit_executors, MEM_LIMIT_EXECUTORS, 
TQueryOptionLevel::ADVANCED)    \
Same as mem_limit


http://gerrit.cloudera.org:8080/#/c/21698/9/be/src/service/query-options.h@326
PS9, Line 326:   QUERY_OPT_FN(mem_limit_coordinators, MEM_LIMIT_COORDINATORS,   
                        \
             :       TQueryOptionLevel::ADVANCED)
I think we should exempt this



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1f4802ad9548749cd43df8848b6f46dca3739ae7
Gerrit-Change-Number: 21698
Gerrit-PatchSet: 9
Gerrit-Owner: Michael Smith <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Kurt Deschler <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Yida Wu <[email protected]>
Gerrit-Comment-Date: Fri, 04 Oct 2024 23:34:29 +0000
Gerrit-HasComments: Yes

Reply via email to