Adar Dembo has posted comments on this change. Change subject: log: release WritableLogSegment buffers when log is idle ......................................................................
Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/6611/3/src/kudu/consensus/log.h File src/kudu/consensus/log.h: PS3, Line 99: Status SwitchToAsyncMode(); > this new state change seems weird because it's one way. could we just refac My issue with an explicit "async mode" is that it exposes the append thread's existence (or non-existence) outside this class. To reduce our overall thread count, an "idle" append thread should exit and a future async operation should start it up again, but that becomes harder to do if the append thread's existence spills out into other classes via this "async mode" thing. I guess if there were a "switch back to sync mode" function then it'd be OK, but ideally this class would manage the starting/stopping of the append thread automatically, without forcing its users to do it. Incidentally, did you explore having the append thread exit outright on idle, and bringing it back up on the next async operation? The synchronization gets hairy... -- To view, visit http://gerrit.cloudera.org:8080/6611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I512c7f9264ac9490e6a57259e4d7b2608adc1ca7 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
