Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15185 )
Change subject: [cpp] KUDU-2971: protobuf-based wrapper for subprocesses ...................................................................... Patch Set 10: (1 comment) http://gerrit.cloudera.org:8080/#/c/15185/9/src/kudu/subprocess/server.h File src/kudu/subprocess/server.h: http://gerrit.cloudera.org:8080/#/c/15185/9/src/kudu/subprocess/server.h@133 PS9, Line 133: > Good points, though I'm hesitant to allow for such dynamic timeouts because Yeah, dynamic timeouts might disrupt the logic. If not allowing for the dynamic timeouts, the change I mentioned makes even more sense :) BTW, if you don't want to allow for changing the timeout in run-time, maybe remove the 'runtime' tag from the --subprocess_timeout_secs flag? Back to the essence: the point was not in allowing dynamic timeouts, but rather storing the deadline for the task upfront (instead of start time). If doing so, there would be no need to access --subprocess_timeout_secs flag value, converting it to MonoDelta, summing it with start_time every 50ms for each task. Less useless work to do, sparing few CPU cycles every 50ms. If you don't buy this, feel free to ignore. -- To view, visit http://gerrit.cloudera.org:8080/15185 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id611e1c683df2721fd058f753b8686a688a5990d Gerrit-Change-Number: 15185 Gerrit-PatchSet: 10 Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Fri, 21 Feb 2020 18:14:40 +0000 Gerrit-HasComments: Yes
