Adar Dembo has posted comments on this change.
Change subject: Add a ResultTracker class that will track server side results
Patch Set 3:
Line 40: lock_guard<simple_spinlock> l(&lock_);
> no, the lock protects access to clients_ and to the ClientState that belong
I suspect it will be, as nearly every RPC will call CheckDuplicate(), no?
Line 78: LOG(FATAL) << "Wrong state";
> the whole point is that we only call this once per RPC, if the caller got N
OK, could we at least log what the wrong state was?
Line 119: ClientState* client_state = FindOrDie(clients_, client_id);
: CompletionRecord* completion_record =
> why? if we are responding to an RPC that is not being tracked then it's an
Well, this goes back to the lack of documentation thing. :)
You're designing a module with "hard" boundaries. That is, if one of us misuses
the ResultTracker, it crashes.
Without context and in isolation, I tend to assume new modules should have
"soft" boundaries, where misuse leads to returned errors. And to be fair, there
are advantages to soft boundaries: it's easier to test such a module in
Anyway, there's nothing inherently wrong with hard boundaries, you just need to
be very clear in the interface regarding what happens in the event of misuse.
To view, visit http://gerrit.cloudera.org:8080/3190
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Owner: David Ribeiro Alves <david.al...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <david.al...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>