Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17725 )
Change subject: KUDU-3204: Add a metric for amount of available space ...................................................................... Patch Set 14: (4 comments) http://gerrit.cloudera.org:8080/#/c/17725/14/src/kudu/server/server_base.cc File src/kudu/server/server_base.cc: http://gerrit.cloudera.org:8080/#/c/17725/14/src/kudu/server/server_base.cc@919 PS14, Line 919: wal_dir_available_space_ I was going to point out that we need to take the lock before returning this. Instead, I'd suggest having RefreshWalDir... return the space to avoid the extra taking of the lock, now that we don't have any output parameters. http://gerrit.cloudera.org:8080/#/c/17725/14/src/kudu/server/server_base.cc@953 PS14, Line 953: MonoTime expiry = wal_dir_space_last_check_ + MonoDelta::FromSeconds( : FLAGS_fs_wal_dir_available_space_cache_seconds); We also need to hold the lock here. Consider early returning when possible to eliminate the extra indentation of added scopes. E.g. void ServerBase::Refresh... { { std::lock_guard<simple_spinlock> l(lock_); if (MonoTime::Now() < wal_dir_space_last_check_ + MonoDelta::FromSeconds(FLAGS...) { return wal_dir_available_space_; } } SpaceInfo space_info_waldir; int64_t wal_dir_space = ... ... std::lock_guard<simple_spinlock> l(lock_); wal_dir_space_last_check_ = MonoTime::Now(); return wal_dir_available_space_; } Same for data dirs http://gerrit.cloudera.org:8080/#/c/17725/14/src/kudu/server/server_base.cc@958 PS14, Line 958: nit: small spacing alignment http://gerrit.cloudera.org:8080/#/c/17725/14/src/kudu/server/server_base.cc@974 PS14, Line 974: & nit: const string& data_dir -- To view, visit http://gerrit.cloudera.org:8080/17725 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I58a7419847d080498aaf431d1dab098e1af71ad0 Gerrit-Change-Number: 17725 Gerrit-PatchSet: 14 Gerrit-Owner: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 26 Aug 2021 23:24:53 +0000 Gerrit-HasComments: Yes
