> 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? > > kturner wrote: > 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.
I believe that the method should return an input stream that is positioned just after the header + crypto opts (ready to read LogEntry's). If OPEN is the first thing that should be in the WAL, that should be the next thing read from the stream. - Josh ----------------------------------------------------------- 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 > >
