Todd Lipcon has posted comments on this change.

Change subject: Replace uses of gutil/atomicops with c++11 atomics
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/1842/1/src/kudu/tablet/tablet_metadata.cc
File src/kudu/tablet/tablet_metadata.cc:

Line 318:         if 
(next_rowset_idx_.compare_exchange_weak(current_rowset_idx, rowset_meta_idx)) {
> next_rowset_idx_ is accessed in CreateRowSet without taking the lock.
but CreateRowSet shouldn't ever be concurrent with LoadFromSuperBlock. again 
would rather just do a straight conversion in this patch instead of changing 
code


Line 599:   Schema* old_schema = schema_.exchange(new_schema.release());
> My comment is saying that it is needed here in order to guarantee that all 
that's what "release" semantics does -- ensures that all previous stores by the 
same thread are made visible before this store. (imagine releasing a mutex -- 
any changes you did while holding the mutex need to be visible to anyone else 
who acquires the mutex)


-- 
To view, visit http://gerrit.cloudera.org:8080/1842
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I249b64ff5efd6c8acbd2c6ff0811f66ff2c4d73f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to