Todd Lipcon has posted comments on this change.
Change subject: Replace boost::{lock, unique_lock, mutex} with std lib
equivalents
......................................................................
Patch Set 2:
(3 comments)
are there any other spots where code changed beyond just find/replace? I found
one that seems to be a bug.
http://gerrit.cloudera.org:8080/#/c/3262/2/src/kudu/tablet/compaction.h
File src/kudu/tablet/compaction.h:
Line 82: const std::shared_ptr<std::unique_lock<std::mutex>>
&lock) {
given unique_lock is moveable, I bet we don't need the shared_ptr anymore
http://gerrit.cloudera.org:8080/#/c/3262/2/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:
Line 1734: std::unique_lock<std::mutex> lock(*rs->compact_flush_lock(),
std::try_to_lock);
this change seems to have broken the scope of this lock - it should be held for
the whole block
http://gerrit.cloudera.org:8080/#/c/3262/2/src/kudu/util/locks.h
File src/kudu/util/locks.h:
Line 234: // Simpler version of std::lock_guard. Only supports the basic object
looking at the implementation of std::lock_guard, I think it's equivalent.
though, looking at boost, I can't quite recall what we used to think was
different about boost's. The commit message that added this says something
about new versions of boost potentially throwing, but I dont see that in the
docs. Either way, maybe we can remove this in a follow-up?
--
To view, visit http://gerrit.cloudera.org:8080/3262
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c27f72c726258793991006a728673af537414bb
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <d...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mpe...@apache.org>
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes
