Binglin Chang has posted comments on this change. Change subject: Add per tablet write RPC throttling ......................................................................
Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/2327/3/src/kudu/tserver/tablet_service.cc File src/kudu/tserver/tablet_service.cc: Line 687: if (!tablet->Throttle(req->ByteSize())) { > - is it more appropriate to use row_operations.rows.size() + row_operations Since batch write is much more efficient than sync write, I think it's reasonable to use number of rpcs than rows. also number of rows is also reflected in bytes, which should have penalty for large batches. Line 690: TabletServerErrorPB::UNKNOWN_ERROR, > let's introduce a new enum like WRITE_THROTTLED here We may also throttle Get(random read) requests in future, so how about just THROTTLED? http://gerrit.cloudera.org:8080/#/c/2327/3/src/kudu/util/throttler.h File src/kudu/util/throttler.h: Line 76: uint64_t num_period = d / kRefillPeriodMicros + 1; > am wondering whether it would make more sense to make this a double (and no why "not +1"? seems incorrect. Also I suspect doing this will has some effect on max token behavior. e.g at 1.05s, add token 15 rather than 10, then the extra 5 token is dropped. -- To view, visit http://gerrit.cloudera.org:8080/2327 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33cb6934d27b883a783682cef1e0723100637d45 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Binglin Chang <[email protected]> Gerrit-Reviewer: Binglin Chang <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
