> On Sept. 30, 2014, 1:32 p.m., kturner wrote: > > server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java, > > line 311 > > <https://reviews.apache.org/r/26153/diff/1/?file=708545#file708545line311> > > > > Could possibly make this check more resiliant by ensuring we can read > > to Open event. That should always be the first thing in log after the > > header (magic and any encryptions props). This would basically handle the > > case where an EOFE occurs while trying to read encryption props. > > Josh Elser wrote: > Yeah, you're right. I didn't think beyond being unable to read the first > header we expect to find but there's a bit more there that could bite us. Do > you think it would be worthwhile to actually read to an Open event and seek > the stream back to just after header+properties?
I am not sure, I did not look to closely at how the whole process works. What state does the code that calls that method expect? Does it expect the stream to be positioned right before the OPEN event? If you don't know off the top of your head, I can look. - kturner ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26153/#review54973 ----------------------------------------------------------- On Sept. 30, 2014, 11:01 p.m., Josh Elser wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26153/ > ----------------------------------------------------------- > > (Updated Sept. 30, 2014, 11:01 p.m.) > > > Review request for accumulo. > > > Bugs: ACCUMULO-3182 > https://issues.apache.org/jira/browse/ACCUMULO-3182 > > > Repository: accumulo > > > Description > ------- > > Ensures that a WAL that was empty or missing a complete header (due to > tservers dying before it could happen) can be successfully recovered (by > assuming that the lack of a complete header means there is no data to > recover). Currently against master, will pull back to 1.5 and merge forward > since it likely affects all branches. > > > Diffs > ----- > > > server/base/src/main/java/org/apache/accumulo/server/log/SortedLogState.java > PRE-CREATION > > server/master/src/main/java/org/apache/accumulo/master/recovery/RecoveryManager.java > 791aec8 > server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java > b4fbfed > server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java > 8de2b25 > server/tserver/src/main/java/org/apache/accumulo/tserver/log/LogSorter.java > 4badefa > > server/tserver/src/main/java/org/apache/accumulo/tserver/log/MultiReader.java > 541f075 > > server/tserver/src/main/java/org/apache/accumulo/tserver/logger/LogReader.java > 681fbd3 > > server/tserver/src/main/java/org/apache/accumulo/tserver/replication/AccumuloReplicaSystem.java > 416a86e > > server/tserver/src/test/java/org/apache/accumulo/tserver/log/MultiReaderTest.java > a79e77e > > server/tserver/src/test/java/org/apache/accumulo/tserver/log/SortedLogRecoveryTest.java > fffa15e > > server/tserver/src/test/java/org/apache/accumulo/tserver/log/TestUpgradePathForWALogs.java > f01ee10 > > test/src/test/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/26153/diff/ > > > Testing > ------- > > New IT, ran UTs and ITs. > > > Thanks, > > Josh Elser > >
