Todd Lipcon has posted comments on this change.

Change subject: master: add read-write lock to serialize operations around 
elections
......................................................................


Patch Set 3:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/3550/3/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

PS3, Line 345:  l.c
could be more descriptive here?


PS3, Line 367: aborting the current task...
hmm, this (old) log message is nonsense, right?


Line 657:     e.second->WaitTasksCompletion();
does this put a restriction on the tasks themselves that they may not block on 
the leader lock? otherwise is there a deadlock case?


http://gerrit.cloudera.org:8080/#/c/3550/3/src/kudu/master/catalog_manager.h
File src/kudu/master/catalog_manager.h:

PS3, Line 304: General status 
a specific example of what types of "bad status" this might represent would be 
useful


PS3, Line 305: leadership_status
you mean leader_status()?
should we DCHECK(catalog_status.ok()) within leader_status() in this case?


PS3, Line 308: Leadership status
would a simple bool suffice?


PS3, Line 316: '.
and returns false


PS3, Line 691: etc
what about read operations? should you specifically say write operations? are 
read operations already protected by the tableinfo locks, etc? or should we 
acquire in them too to prevent a race where the Visit*() functions clear the 
maps just as a read request arrives (but after the read request checks 
leadership)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5084c09f1a77ccf620fb6cd621094c4778d636f8
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <d...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to