Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12037 )

Change subject: IMPALA-7928: Consistent remote read scheduling
......................................................................


Patch Set 8: Code-Review+2

(3 comments)

I'm comfortable with this. Nice work!

I'm still thinking a bit about end-to-end testing here. I think we should keep 
thinking about Tim's docker work to see if we can have a style of test that 
create remotes read because of containers. We can delay that.

http://gerrit.cloudera.org:8080/#/c/12037/5/be/src/scheduling/hash-ring.h
File be/src/scheduling/hash-ring.h:

http://gerrit.cloudera.org:8080/#/c/12037/5/be/src/scheduling/hash-ring.h@106
PS5, Line 106:   /// impact the actual allocations.
> I think doing some work to reduce our use of strings in the scheduler would
I've not seen evidence that scheduling is slow at the moment; I think it's fine 
to leave this alone.


http://gerrit.cloudera.org:8080/#/c/12037/5/be/src/scheduling/hash-ring.h@111
PS5, Line 111: };
> Two thoughts here:
I thought about this in the previous review. I don't think it's a big deal. The 
scheduler tries to spread out the load across replicas. If a block has 1 or 2 
fake replicas instead of 3, the scheduler will be able to even it out.


http://gerrit.cloudera.org:8080/#/c/12037/5/be/src/scheduling/scheduler.h
File be/src/scheduling/scheduler.h:

http://gerrit.cloudera.org:8080/#/c/12037/5/be/src/scheduling/scheduler.h@191
PS5, Line 191:     /// the file name / offset from 'hdfs_file_split' multiple 
times and look up the
> That is a good idea. We want big files to get split up, and this shouldn't
Great find, Lars.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icbf74088a8bd8c285ab7285ea3a01acd1bb53a45
Gerrit-Change-Number: 12037
Gerrit-PatchSet: 8
Gerrit-Owner: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Lars Volker <[email protected]>
Gerrit-Reviewer: Michael Ho <[email protected]>
Gerrit-Reviewer: Philip Zeyliger <[email protected]>
Gerrit-Comment-Date: Tue, 05 Feb 2019 17:38:10 +0000
Gerrit-HasComments: Yes

Reply via email to