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