David Ribeiro Alves has posted comments on this change. Change subject: Integrate the ResultTracker into the rpc subsystem ......................................................................
Patch Set 24: (7 comments) http://gerrit.cloudera.org:8080/#/c/3192/24/src/kudu/rpc/protoc-gen-krpc.cc File src/kudu/rpc/protoc-gen-krpc.cc: Line 456: "result_tracker_ = result_tracker;" > this can use an initializer list, right? well, this is initializing a protected member of a superclass, to be able to use an initializer list I'd need to add a new ctor the and then call that ctor from here. Mind if we just leave it? http://gerrit.cloudera.org:8080/#/c/3192/24/src/kudu/rpc/rpc-stress-test.cc File src/kudu/rpc/rpc-stress-test.cc: PS24, Line 59: We don't use CalculatorServiceRpc > not sure what class this is referring to. I guess this is coming in a later I can't remember either. All of them use that service. removed. Line 68: atomic_int* attempt_nos) { > not following why this is atomic. you're fetch_add()ing to it from the main Done Line 70: client_sleep_for_ms = client_sleep; > above two lines can be from an initializer list Done PS24, Line 83: a the > typo Done Line 91: uint64_t client_sleep_for_ms; > make these above two const Done Line 100: string client_id_; > this is just a constant, right? can you just make it a const char* kClientI Done -- To view, visit http://gerrit.cloudera.org:8080/3192 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1d624810350feceefe244e0319e22cac241bf0d6 Gerrit-PatchSet: 24 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves <dral...@apache.org> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
