Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/17903 )
Change subject: KUDU-1959 - Implement aggregate startup progress metrics ...................................................................... Patch Set 5: (5 comments) http://gerrit.cloudera.org:8080/#/c/17903/5//COMMIT_MSG Commit Message: PS5: > It's probably worth adding a small test for this, or maybe reusing some of Will add a test for these metrics in the exisitng tests patch. http://gerrit.cloudera.org:8080/#/c/17903/5//COMMIT_MSG@9 PS5, Line 9: We expose the below metrics as a part of this commit: : * startup_progress_steps_remaining : count of server startup steps which : are yet to be completed. This value is in the range [0,4]. : * startup_progress_time_elapsed : the time elapsed so far for the server to : startup. If the startup is completed, this is the total time taken for the : startup. This is in milliseconds. : These metrics are primarily expected to be used by third party monitoring tools : to see how long has the server taken to startup historically for any sort of : trend analysis. : The startup_progress_time_elapsed metric can also be used to check the : previous startup time as an alternative to the startup page in the WebUI. > style nit: it's personal preference, but it'd be nice if this weren't so co Done http://gerrit.cloudera.org:8080/#/c/17903/5/src/kudu/server/startup_path_handler.h File src/kudu/server/startup_path_handler.h: http://gerrit.cloudera.org:8080/#/c/17903/5/src/kudu/server/startup_path_handler.h@57 PS5, Line 57: int32 > nit: just use an int Done http://gerrit.cloudera.org:8080/#/c/17903/5/src/kudu/server/startup_path_handler.cc File src/kudu/server/startup_path_handler.cc: http://gerrit.cloudera.org:8080/#/c/17903/5/src/kudu/server/startup_path_handler.cc@143 PS5, Line 143: int counter = (init_progress_.IsStopped() ? 0 : 1) : + (read_filesystem_progress_.IsStopped() ? 0 : 1) : + (is_tablet_server_ ? (start_tablets_progress_.IsStopped() ? 0 : 1) : 0) : + (is_tablet_server_ ? 0 : : (initialize_master_catalog_progress_.IsStopped() ? 0 : 1)) : + (start_rpc_server_progress_.IsStopped() ? 0 : 1); > nit: IMO would be more easily read as Done http://gerrit.cloudera.org:8080/#/c/17903/5/src/kudu/server/startup_path_handler.cc@152 PS5, Line 152: int64_t > nit: consider returning a MonoDelta, and doing the millisecond conversion a Done -- To view, visit http://gerrit.cloudera.org:8080/17903 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0a508c3baf0a0d77baf75f36f7bb305a6ad821e1 Gerrit-Change-Number: 17903 Gerrit-PatchSet: 5 Gerrit-Owner: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 27 Oct 2021 16:06:36 +0000 Gerrit-HasComments: Yes
