Adar Dembo has posted comments on this change.

Change subject: WIP: KUDU-1713: add a client Partitioner API
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5775/2/src/kudu/client/client.h
File src/kudu/client/client.h:

Line 2216:   Status PartitionRow(const KuduPartialRow& row, int* partition);
> The second approach is still coupled to the metacache, which could be arbit
If your concern is concurrent changes, I don't think either approach provides 
true safety. The first approach does narrows the window of vulnerability to the 
loop inside KuduPartitionerBuilder::Build(), but it doesn't eliminate it 
altogether.

But, your other point (about mapping to partition numbers) is valid, and I can 
see how the first approach models the KuduPartitioner as a "view" of the 
metacache. I can also see how, in the future, we could introduce some kind of 
locking so that the loop in KuduPartitionerBuilder::Build() could be completely 
safe.

OK. I'm convinced.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic08a078c75f15ef4200219b5260cfb99d79b72cc
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <t...@apache.org>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to