Todd Lipcon 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? 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 patch? Line 68: atomic_int* attempt_nos) { not following why this is atomic. you're fetch_add()ing to it from the main thread in the constructor, and then it's not accessed from the started threads. Why not just take an int attempt_num here? Line 70: client_sleep_for_ms = client_sleep; above two lines can be from an initializer list PS24, Line 83: a the typo Line 91: uint64_t client_sleep_for_ms; make these above two const Line 100: string client_id_; this is just a constant, right? can you just make it a const char* kClientId? -- 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
