You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@accumulo.apache.org by Josh Elser <jo...@gmail.com> on 2014/09/30 00:01:48 UTC

Review Request 26153: ACCUMULO-3182 WAL recovery on empty files doesn't fail

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26153/
-----------------------------------------------------------

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, running existing unit tests right now. Will run ITs.


Thanks,

Josh Elser


Re: Review Request 26153: ACCUMULO-3182 WAL recovery on empty files doesn't fail

Posted by Josh Elser <jo...@gmail.com>.

> On Sept. 30, 2014, 8:50 p.m., Eric Newton wrote:
> > server/tserver/src/main/java/org/apache/accumulo/tserver/log/MultiReader.java, line 98
> > <https://reviews.apache.org/r/26153/diff/2/?file=709763#file709763line98>
> >
> >     Can you make a little method for making/checking these markers?  The line is getting a little long/ugly.

Sure, easy enough to add something to the enum to encapsulate the changes.


- Josh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26153/#review55005
-----------------------------------------------------------


On Sept. 30, 2014, 8:28 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, 8:28 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
> 
>


Re: Review Request 26153: ACCUMULO-3182 WAL recovery on empty files doesn't fail

Posted by ke...@deenlo.com.

> On Sept. 30, 2014, 8:50 p.m., Eric Newton wrote:
> > test/src/test/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java, line 162
> > <https://reviews.apache.org/r/26153/diff/2/?file=709769#file709769line162>
> >
> >     Nice test

I'll second that.  Nice tests!


- kturner


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26153/#review55005
-----------------------------------------------------------


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
> 
>


Re: Review Request 26153: ACCUMULO-3182 WAL recovery on empty files doesn't fail

Posted by Eric Newton <er...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26153/#review55005
-----------------------------------------------------------



server/tserver/src/main/java/org/apache/accumulo/tserver/log/MultiReader.java
<https://reviews.apache.org/r/26153/#comment95325>

    Can you make a little method for making/checking these markers?  The line is getting a little long/ugly.



server/tserver/src/test/java/org/apache/accumulo/tserver/log/MultiReaderTest.java
<https://reviews.apache.org/r/26153/#comment95327>

    A little method would clean up the duplicate code.



test/src/test/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java
<https://reviews.apache.org/r/26153/#comment95331>

    Nice test



test/src/test/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java
<https://reviews.apache.org/r/26153/#comment95330>

    Nice test


- Eric Newton


On Sept. 30, 2014, 8:28 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, 8:28 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
> 
>


Re: Review Request 26153: ACCUMULO-3182 WAL recovery on empty files doesn't fail

Posted by Josh Elser <jo...@gmail.com>.

> On Oct. 1, 2014, 12:48 a.m., kturner wrote:
> > server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java, line 313
> > <https://reviews.apache.org/r/26153/diff/1-2/?file=708545#file708545line313>
> >
> >     what about EOF check here?  Why have multiple try/catch blocks for EOFE, why not one try/catch block that covers a lot of code?

Yeah, you're right. I totally missed this one. I was being obstinate against just throwing a big try/catch around it -- that would be much simpler in the long run.


- Josh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26153/#review55040
-----------------------------------------------------------


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
> 
>


Re: Review Request 26153: ACCUMULO-3182 WAL recovery on empty files doesn't fail

Posted by ke...@deenlo.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26153/#review55040
-----------------------------------------------------------



server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java
<https://reviews.apache.org/r/26153/#comment95371>

    what about EOF check here?  Why have multiple try/catch blocks for EOFE, why not one try/catch block that covers a lot of code?


- kturner


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
> 
>


Re: Review Request 26153: ACCUMULO-3182 WAL recovery on empty files doesn't fail

Posted by Josh Elser <jo...@gmail.com>.

> On Oct. 1, 2014, 1:02 a.m., kturner wrote:
> > test/src/test/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java, line 196
> > <https://reviews.apache.org/r/26153/diff/3/?file=709899#file709899line196>
> >
> >     Could use the following here...
> >     
> >     MetadataSchema.TabletsSection.getRow(tableId, null);

Ah, thanks! I missed that helpful method.


> On Oct. 1, 2014, 1:02 a.m., kturner wrote:
> > test/src/test/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java, line 216
> > <https://reviews.apache.org/r/26153/diff/3/?file=709899#file709899line216>
> >
> >     this sleep should not be needed

I agree with you, but I still see https://issues.apache.org/jira/browse/ACCUMULO-2964 infrequently. Offline'ing the table should actually preclude the unexplained Thrift exception (if I'm right in that it's tied to the tserver restart) and it would actually make the test more generally usable.


- Josh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26153/#review55044
-----------------------------------------------------------


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
> 
>


Re: Review Request 26153: ACCUMULO-3182 WAL recovery on empty files doesn't fail

Posted by ke...@deenlo.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26153/#review55044
-----------------------------------------------------------



test/src/test/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java
<https://reviews.apache.org/r/26153/#comment95374>

    Could use the following here...
    
    MetadataSchema.TabletsSection.getRow(tableId, null);



test/src/test/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java
<https://reviews.apache.org/r/26153/#comment95377>

    I wonder if the test would work and run faster if the table was taken offline, metadata table modified, and then table brought back online.  I have no problem w/ this approach, this was just something I thought of..



test/src/test/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java
<https://reviews.apache.org/r/26153/#comment95376>

    this sleep should not be needed


- kturner


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
> 
>


Re: Review Request 26153: ACCUMULO-3182 WAL recovery on empty files doesn't fail

Posted by Josh Elser <jo...@gmail.com>.

> On Oct. 2, 2014, 3:49 p.m., kturner wrote:
> > test/src/test/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java, line 75
> > <https://reviews.apache.org/r/26153/diff/3-4/?file=709899#file709899line75>
> >
> >     This is funny.   Was this something you only saw after switching to offline/online?  I suppose wacking the tserver kept gc from running.

Haha yup. Every ~1/10 runs of the test case, I'd get a FileNotFoundException when the tserver would try to read the file. Best I could guess is that the GC cleaned up the file because it looked like a WAL and wasn't referenced at all in metadata. :)


- Josh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26153/#review55209
-----------------------------------------------------------


On Oct. 1, 2014, 7:15 p.m., Josh Elser wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26153/
> -----------------------------------------------------------
> 
> (Updated Oct. 1, 2014, 7:15 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
> 
>


Re: Review Request 26153: ACCUMULO-3182 WAL recovery on empty files doesn't fail

Posted by ke...@deenlo.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26153/#review55209
-----------------------------------------------------------

Ship it!



test/src/test/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java
<https://reviews.apache.org/r/26153/#comment95599>

    This is funny.   Was this something you only saw after switching to offline/online?  I suppose wacking the tserver kept gc from running.


- kturner


On Oct. 1, 2014, 7:15 p.m., Josh Elser wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26153/
> -----------------------------------------------------------
> 
> (Updated Oct. 1, 2014, 7:15 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
> 
>


Re: Review Request 26153: ACCUMULO-3182 WAL recovery on empty files doesn't fail

Posted by Josh Elser <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26153/
-----------------------------------------------------------

(Updated Oct. 1, 2014, 7:15 p.m.)


Review request for accumulo.


Changes
-------

* Lifted the try/catch(EOFException) around main body of DfsLogger.readHeadeAndReturnStream
* Fixed an NPE in LogSorter.close when we get one of the WALs
* Used offline/online to trigger recovery on the table
* Used more MetadataSchema helper methods
* Added a delay to the GC start as it would sometimes remove the file we created before the metadata reference was added


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 (updated)
-----

  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


Re: Review Request 26153: ACCUMULO-3182 WAL recovery on empty files doesn't fail

Posted by Josh Elser <jo...@gmail.com>.
-----------------------------------------------------------
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.


Changes
-------

Incorporated feedback from Eric


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 (updated)
-----

  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


Re: Review Request 26153: ACCUMULO-3182 WAL recovery on empty files doesn't fail

Posted by Josh Elser <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26153/
-----------------------------------------------------------

(Updated Sept. 30, 2014, 8:28 p.m.)


Review request for accumulo.


Changes
-------

Updated patch to be a little more resilient from Keith's advice. Added an extra test case I meant to add before the first patch


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 (updated)
-----

  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


Re: Review Request 26153: ACCUMULO-3182 WAL recovery on empty files doesn't fail

Posted by ke...@deenlo.com.

> 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
> 
>


Re: Review Request 26153: ACCUMULO-3182 WAL recovery on empty files doesn't fail

Posted by Josh Elser <jo...@gmail.com>.

> 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.

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?


- Josh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26153/#review54973
-----------------------------------------------------------


On Sept. 30, 2014, 4:29 a.m., Josh Elser wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26153/
> -----------------------------------------------------------
> 
> (Updated Sept. 30, 2014, 4:29 a.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
> 
>


Re: Review Request 26153: ACCUMULO-3182 WAL recovery on empty files doesn't fail

Posted by Josh Elser <jo...@gmail.com>.

> 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
> 
>


Re: Review Request 26153: ACCUMULO-3182 WAL recovery on empty files doesn't fail

Posted by ke...@deenlo.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26153/#review54973
-----------------------------------------------------------



server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java
<https://reviews.apache.org/r/26153/#comment95303>

    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.


- kturner


On Sept. 30, 2014, 4:29 a.m., Josh Elser wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26153/
> -----------------------------------------------------------
> 
> (Updated Sept. 30, 2014, 4:29 a.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
> 
>


Re: Review Request 26153: ACCUMULO-3182 WAL recovery on empty files doesn't fail

Posted by Josh Elser <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26153/
-----------------------------------------------------------

(Updated Sept. 30, 2014, 4:29 a.m.)


Review request for accumulo.


Changes
-------

Updated testing


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 (updated)
-------

New IT, ran UTs and ITs.


Thanks,

Josh Elser