> On Jan. 21, 2014, 7:28 p.m., Mike Drob wrote: > > src/server/src/main/java/org/apache/accumulo/server/trace/TraceServer.java, > > line 135 > > <https://reviews.apache.org/r/17129/diff/2/?file=433578#file433578line135> > > > > 'writer == null' would minimize the diff.
line still changes because of the addition of the curly to allow the log. > On Jan. 21, 2014, 7:28 p.m., Mike Drob wrote: > > src/server/src/main/java/org/apache/accumulo/server/trace/TraceServer.java, > > line 140 > > <https://reviews.apache.org/r/17129/diff/2/?file=433578#file433578line140> > > > > Why aren't we using guards? I had convinced myself earlier that it is > > because that could inadvertantly cause traces to block, but I'm not > > convinced that is a good reason. I presumed it was because of the added overhead in handling a lock when traces are incoming. Esp. since BW itself is threadsafe, it's just more contention. Maybe the answer is that BatchWriter itself should have a "reset yourself" method? > On Jan. 21, 2014, 7:28 p.m., Mike Drob wrote: > > src/server/src/main/java/org/apache/accumulo/server/trace/TraceServer.java, > > line 153 > > <https://reviews.apache.org/r/17129/diff/2/?file=433578#file433578line153> > > > > Maybe worth catching NPE separately? Any particular reason? FWIW, the exception that caused me to add the block was an IllegalArgumentException because of the writer being closed. I just noticed that NPE could also happen. Just to avoid the stack trace, since we know in that case it's the writer? what if it happened within the addmutation call? - Sean ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17129/#review32403 ----------------------------------------------------------- On Jan. 21, 2014, 7:24 p.m., Sean Busbey wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17129/ > ----------------------------------------------------------- > > (Updated Jan. 21, 2014, 7:24 p.m.) > > > Review request for accumulo and Eric Newton. > > > Bugs: ACCUMULO-2213 > https://issues.apache.org/jira/browse/ACCUMULO-2213 > > > Repository: accumulo > > > Description > ------- > > ACCUMULO-2213 Make writer recovery in the Tracer more robust. > > * Adds optional logging to TSBW closure so we can better instrument long > run tests to look for problems. > * Cleans up writer reseting in the TraceServer, avoids overly broad > catching. > * tones down log levels in TraceServer to WARN because trace information > is transient and we retry everything. > > > Diffs > ----- > > > src/core/src/main/java/org/apache/accumulo/core/client/impl/TabletServerBatchWriter.java > e18f3f96f4e0959b975399921bb7d7958c561220 > src/server/src/main/java/org/apache/accumulo/server/trace/TraceServer.java > 4d89e9c13f89e71e674875db32cf267e045d6131 > > Diff: https://reviews.apache.org/r/17129/diff/ > > > Testing > ------- > > running on cluster with injected failures now > > > Thanks, > > Sean Busbey > >
