Adar Dembo has posted comments on this change.
Change subject: log: Mark allocation finished even if allocation had an error
Patch Set 1:
Line 206: LOG(FATAL) << "Error syncing log" << s.ToString();
> Why isn't this one DFATAL? The old code was DLOG(FATAL), which I think is e
We talked about this on Slack. I now understand that your goal was to induce a
crash here (and presumably above) in the event that appending or syncing fails.
I strongly disagree with this. I empathize with the use case (that a leader
replica that cannot write to its WAL should be killed), but this isn't the
right layer for that. If we need to induce a crash when a leader can't write to
its WAL, we should do that somewhere in the consensus module where we've
checked that the replica is a leader and not a follower.
In general, we're doing a poor job of isolating Raft consensus details to the
consensus module. They're spilling out all over the codebase and it's making
Kudu as a project more difficult to maintain. I'm not trying to pick on you for
this; obviously we're all to blame. But let's think very carefully before we
introduce more logic that brings a Raft detail into a module where it
(ostensibly) doesn't belong.
To view, visit http://gerrit.cloudera.org:8080/3234
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Owner: Mike Percy <mpe...@apache.org>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mpe...@apache.org>
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>