Alexey Serbin has posted comments on this change.

Change subject: threadpool: token-based task sequencing
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6874/4/src/kudu/util/threadpool.cc
File src/kudu/util/threadpool.cc:

PS4, Line 345:     unique_ptr<SequenceSlot> new_slot(new SequenceSlot(&lock_));
             :     DCHECK_EQ(SequenceSlot::DEALLOCATED, new_slot->state());
             :     slots_.emplace_back(std::move(new_slot));
             :     idx = slots_.size() - 1;
             :   }
             : 
             :   slots_[idx]->Transition(SequenceSlot::INACTIVE);
> So why do we actually need both DEALLOCATED and INACTIVE? Because we need t
ok, I see: the broadcasting on conditional variable is no-op in that case.  
Thank you for the clarification.  Yes, the new names for the states in PS6 look 
better than the former ones.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If46dc34212027b6ea5dbc2ead7c7af8be57f2c70
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <danburk...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <davidral...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to