Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12987 )

Change subject: IMPALA-8341: Data cache for remote reads
......................................................................


Patch Set 4:

(3 comments)

I don't know that I'll have time to do a fully detailed review, and it looks 
like we may already have enough eyes on it. The design and APIs look great - it 
does seem like we want to get it checked in and exercised more soon.

http://gerrit.cloudera.org:8080/#/c/12987/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/12987/4//COMMIT_MSG@61
PS4, Line 61: Testing done: a new BE test was added; core test with cache 
enabled.
Can we add a custom cluster test to sanity check that it works end to end. 
Myabe this is where we should sanity check metrics too.


http://gerrit.cloudera.org:8080/#/c/12987/4//COMMIT_MSG@62
PS4, Line 62:
Do we want to consider enabling this by default for the dockerised tests as a 
next step?


http://gerrit.cloudera.org:8080/#/c/12987/4/be/src/runtime/io/data-cache.h
File be/src/runtime/io/data-cache.h:

http://gerrit.cloudera.org:8080/#/c/12987/4/be/src/runtime/io/data-cache.h@88
PS4, Line 88: /// - bound number of backing files per partition by 
consolidating the content of very
> Should we just delete old files if we have reached a configurable high numb
Yeah this scenario is a bit concerning for me still since it's conceivable that 
the workloads we test with might have different properties to real ones. E.g. I 
can imagine a workload might end up with some set of files that are queries 
frequently enough to stay resident in the cache indefinitely and keep very old 
files alive. Not sure if Lars' idea is easier than compaction but it is 
appealing in some ways.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I734803c1c1787c858dc3ffa0a2c0e33e77b12edc
Gerrit-Change-Number: 12987
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Ho <k...@cloudera.com>
Gerrit-Reviewer: David Rorke <dro...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com>
Gerrit-Reviewer: Lars Volker <l...@cloudera.com>
Gerrit-Reviewer: Michael Ho <k...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <stak...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <tmarsh...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-Comment-Date: Tue, 23 Apr 2019 23:38:20 +0000
Gerrit-HasComments: Yes

Reply via email to