David Ribeiro Alves has posted comments on this change.

Change subject: Add unique id generation to the client
......................................................................


Patch Set 8:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/3077/8//COMMIT_MSG
Commit Message:

Line 10: an uuid. The uuid is genrated by the builder, specifically by a static
> Nit: generated
Done


Line 13: This also adds a test that generated 1000 clients and makes sure that
> Nit: "generates" (since you're writing in the present tense).
Done


Line 14: Since this test will now run all the time
       : we should get notified if stuff becomes flaky dues to repeated ids.
> Is this a real concern, though? Don't we use OidGenerators elsewhere withou
well a couple of things. first if we ever break id generation for some reason 
this will catch it (already did once) and if it ever fails we'll log and track 
it. so it seems like it's worth it, particularly as the test is fast.


http://gerrit.cloudera.org:8080/#/c/3077/8/src/kudu/client/client.h
File src/kudu/client/client.h:

Line 255:   const std::string& client_id() const;
> Why expose it to users at all, though? How is it useful to them? I thought 
Done


http://gerrit.cloudera.org:8080/#/c/3077/8/src/kudu/client/client_builder-internal.h
File src/kudu/client/client_builder-internal.h:

Line 24: #include "kudu/util/oid_generator.h"
> Don't need this here; it's only used in the .cc.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie752ab8337cc76c3be1536958c24a8b6c5794650
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: David Ribeiro Alves <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to