You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@accumulo.apache.org by Eric Newton <er...@gmail.com> on 2015/03/19 00:58:06 UTC

Review Request 32224: ACCUMULO-3423

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

Review request for accumulo.


Repository: accumulo


Description
-------

Faster WAL rollovers


Diffs
-----

  core/src/main/java/org/apache/accumulo/core/client/impl/ReplicationOperationsImpl.java 6a5c74a 
  core/src/main/java/org/apache/accumulo/core/conf/Property.java 2403915 
  core/src/main/java/org/apache/accumulo/core/metadata/RootTable.java 24148b1 
  core/src/main/java/org/apache/accumulo/core/metadata/schema/MetadataSchema.java 534dd7f 
  core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java 25d0f32 
  core/src/test/java/org/apache/accumulo/core/metadata/MetadataTableSchemaTest.java PRE-CREATION 
  server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java fc26ca4 
  server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java e5bdded 
  server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataStateStore.java 7ee6f0c 
  server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataTableScanner.java 9be5a67 
  server/base/src/main/java/org/apache/accumulo/server/master/state/TabletLocationState.java b24b562 
  server/base/src/main/java/org/apache/accumulo/server/master/state/TabletStateStore.java 5413e31 
  server/base/src/main/java/org/apache/accumulo/server/master/state/ZooTabletStateStore.java ab99396 
  server/base/src/main/java/org/apache/accumulo/server/replication/StatusUtil.java 898e3d4 
  server/base/src/main/java/org/apache/accumulo/server/util/ListVolumesUsed.java e90d1dd 
  server/base/src/main/java/org/apache/accumulo/server/util/MasterMetadataUtil.java 4ca2d64 
  server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java 10cd749 
  server/base/src/main/java/org/apache/accumulo/server/util/ReplicationTableUtil.java af02a8d 
  server/base/src/test/java/org/apache/accumulo/server/util/ReplicationTableUtilTest.java b1010c2 
  server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java 1735c0d 
  server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java c8d5cd6 
  server/gc/src/main/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferences.java 3a32727 
  server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogsTest.java 5801faa 
  server/gc/src/test/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferencesTest.java 23db83a 
  server/master/src/main/java/org/apache/accumulo/master/Master.java 3762f32 
  server/master/src/main/java/org/apache/accumulo/master/MasterClientServiceHandler.java f73c236 
  server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java d097d75 
  server/master/src/main/java/org/apache/accumulo/master/replication/WorkMaker.java 8532e1b 
  server/master/src/main/java/org/apache/accumulo/master/state/MergeStats.java 44f229e 
  server/master/src/test/java/org/apache/accumulo/master/ReplicationOperationsImplTest.java a127dcd 
  server/master/src/test/java/org/apache/accumulo/master/TestMergeState.java b0240f1 
  server/master/src/test/java/org/apache/accumulo/master/state/RootTabletStateStoreTest.java abceae4 
  server/tserver/src/main/findbugs/exclude-filter.xml 47dd1f5 
  server/tserver/src/main/java/org/apache/accumulo/server/GarbageCollectionLogger.java 8f3785e 
  server/tserver/src/main/java/org/apache/accumulo/tserver/TabletLevel.java PRE-CREATION 
  server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java 662ee31 
  server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java 5acf5eb 
  server/tserver/src/main/java/org/apache/accumulo/tserver/log/SortedLogRecovery.java c4d9fab 
  server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java 711c497 
  server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CommitSession.java b4814e4 
  server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/DatafileManager.java 594d9c5 
  server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java 6152500 
  server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/TabletCommitter.java 4bc05a6 
  test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java b429607 
  test/src/test/java/org/apache/accumulo/proxy/ProxyDurabilityIT.java 404a8fd 
  test/src/test/java/org/apache/accumulo/test/BadDeleteMarkersCreatedIT.java 25337b2 
  test/src/test/java/org/apache/accumulo/test/BalanceIT.java f793925 
  test/src/test/java/org/apache/accumulo/test/CleanWalIT.java f553be8 
  test/src/test/java/org/apache/accumulo/test/ConditionalWriterIT.java bd00f02 
  test/src/test/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java b78a311 
  test/src/test/java/org/apache/accumulo/test/NoMutationRecoveryIT.java 6a9975c 
  test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 56a6a70 
  test/src/test/java/org/apache/accumulo/test/functional/WALSunnyDayIT.java PRE-CREATION 
  test/src/test/java/org/apache/accumulo/test/functional/WatchTheWatchCountIT.java bd0555b 
  test/src/test/java/org/apache/accumulo/test/performance/RollWALPerformanceIT.java PRE-CREATION 
  test/src/test/java/org/apache/accumulo/test/replication/GarbageCollectorCommunicatesWithTServersIT.java 5b89d9c 
  test/src/test/java/org/apache/accumulo/test/replication/MultiInstanceReplicationIT.java 9dec16e 
  test/src/test/java/org/apache/accumulo/test/replication/ReplicationIT.java 54348db 

Diff: https://reviews.apache.org/r/32224/diff/


Testing
-------

Ran all tests, except RandomWalk.


Thanks,

Eric Newton


Re: Review Request 32224: ACCUMULO-3423

Posted by Eric Newton <er...@gmail.com>.

> On March 19, 2015, 8:45 p.m., Josh Elser wrote:
> > core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java, line 36
> > <https://reviews.apache.org/r/32224/diff/1/?file=899476#file899476line36>
> >
> >     As a follow-on, this would be a good class to get some explicit unit test coverage for (since it acts less like an internal struct with your changes)

Created a unit test for LogEntry


> On March 19, 2015, 8:45 p.m., Josh Elser wrote:
> > core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java, line 109
> > <https://reviews.apache.org/r/32224/diff/1/?file=899476#file899476line109>
> >
> >     Have you tested out a dirty 1.6 shutdown and starting up 1.7 with these changes in place?

Yes, I've started this up on a dirty 1.6-SNAPSHOT


> On March 19, 2015, 8:45 p.m., Josh Elser wrote:
> > server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java, line 77
> > <https://reviews.apache.org/r/32224/diff/1/?file=899508#file899508line77>
> >
> >     Why is DfsLogger Comparable now? Isn't the log name still just a UUID?

DfsLogger is now stored in a ConcurrentSkipList which needs it to be Comparable.


- Eric


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


On March 19, 2015, 4:42 p.m., Eric Newton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32224/
> -----------------------------------------------------------
> 
> (Updated March 19, 2015, 4:42 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-3423
>     https://issues.apache.org/jira/browse/ACCUMULO-3423
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Faster WAL rollovers
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/client/impl/ReplicationOperationsImpl.java 6a5c74a 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java 2403915 
>   core/src/main/java/org/apache/accumulo/core/metadata/RootTable.java 24148b1 
>   core/src/main/java/org/apache/accumulo/core/metadata/schema/MetadataSchema.java 534dd7f 
>   core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java 25d0f32 
>   core/src/test/java/org/apache/accumulo/core/metadata/MetadataTableSchemaTest.java PRE-CREATION 
>   server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java fc26ca4 
>   server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java e5bdded 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataStateStore.java 7ee6f0c 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataTableScanner.java 9be5a67 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/TabletLocationState.java b24b562 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/TabletStateStore.java 5413e31 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/ZooTabletStateStore.java ab99396 
>   server/base/src/main/java/org/apache/accumulo/server/replication/StatusUtil.java 898e3d4 
>   server/base/src/main/java/org/apache/accumulo/server/util/ListVolumesUsed.java e90d1dd 
>   server/base/src/main/java/org/apache/accumulo/server/util/MasterMetadataUtil.java 4ca2d64 
>   server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java 10cd749 
>   server/base/src/main/java/org/apache/accumulo/server/util/ReplicationTableUtil.java af02a8d 
>   server/base/src/test/java/org/apache/accumulo/server/util/ReplicationTableUtilTest.java b1010c2 
>   server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java 1735c0d 
>   server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java c8d5cd6 
>   server/gc/src/main/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferences.java 3a32727 
>   server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogsTest.java 5801faa 
>   server/gc/src/test/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferencesTest.java 23db83a 
>   server/master/src/main/java/org/apache/accumulo/master/Master.java 3762f32 
>   server/master/src/main/java/org/apache/accumulo/master/MasterClientServiceHandler.java f73c236 
>   server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java d097d75 
>   server/master/src/main/java/org/apache/accumulo/master/replication/WorkMaker.java 8532e1b 
>   server/master/src/main/java/org/apache/accumulo/master/state/MergeStats.java 44f229e 
>   server/master/src/test/java/org/apache/accumulo/master/ReplicationOperationsImplTest.java a127dcd 
>   server/master/src/test/java/org/apache/accumulo/master/TestMergeState.java b0240f1 
>   server/master/src/test/java/org/apache/accumulo/master/state/RootTabletStateStoreTest.java abceae4 
>   server/tserver/src/main/findbugs/exclude-filter.xml 47dd1f5 
>   server/tserver/src/main/java/org/apache/accumulo/server/GarbageCollectionLogger.java 8f3785e 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletLevel.java PRE-CREATION 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java 662ee31 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java 5acf5eb 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/SortedLogRecovery.java c4d9fab 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java 711c497 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CommitSession.java b4814e4 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/DatafileManager.java 594d9c5 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java 6152500 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/TabletCommitter.java 4bc05a6 
>   test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java b429607 
>   test/src/test/java/org/apache/accumulo/proxy/ProxyDurabilityIT.java 404a8fd 
>   test/src/test/java/org/apache/accumulo/test/BadDeleteMarkersCreatedIT.java 25337b2 
>   test/src/test/java/org/apache/accumulo/test/BalanceIT.java f793925 
>   test/src/test/java/org/apache/accumulo/test/CleanWalIT.java f553be8 
>   test/src/test/java/org/apache/accumulo/test/ConditionalWriterIT.java bd00f02 
>   test/src/test/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java b78a311 
>   test/src/test/java/org/apache/accumulo/test/NoMutationRecoveryIT.java 6a9975c 
>   test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 56a6a70 
>   test/src/test/java/org/apache/accumulo/test/functional/WALSunnyDayIT.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/functional/WatchTheWatchCountIT.java bd0555b 
>   test/src/test/java/org/apache/accumulo/test/performance/RollWALPerformanceIT.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/replication/GarbageCollectorCommunicatesWithTServersIT.java 5b89d9c 
>   test/src/test/java/org/apache/accumulo/test/replication/MultiInstanceReplicationIT.java 9dec16e 
>   test/src/test/java/org/apache/accumulo/test/replication/ReplicationIT.java 54348db 
> 
> Diff: https://reviews.apache.org/r/32224/diff/
> 
> 
> Testing
> -------
> 
> Ran all tests, except RandomWalk.
> 
> 
> Thanks,
> 
> Eric Newton
> 
>


Re: Review Request 32224: ACCUMULO-3423

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



core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java
<https://reviews.apache.org/r/32224/#comment124953>

    As a follow-on, this would be a good class to get some explicit unit test coverage for (since it acts less like an internal struct with your changes)



core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java
<https://reviews.apache.org/r/32224/#comment124954>

    Have you tested out a dirty 1.6 shutdown and starting up 1.7 with these changes in place?



server/base/src/main/java/org/apache/accumulo/server/master/state/TabletLocationState.java
<https://reviews.apache.org/r/32224/#comment124955>

    just delete it



server/base/src/main/java/org/apache/accumulo/server/replication/StatusUtil.java
<https://reviews.apache.org/r/32224/#comment124956>

    It would be better to remove the synchronized block and make a new builder each time. This is more in line with how PB is intended to be used.



server/base/src/main/java/org/apache/accumulo/server/util/MasterMetadataUtil.java
<https://reviews.apache.org/r/32224/#comment124957>

    Add a better msg if you intend to keep it in place



server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java
<https://reviews.apache.org/r/32224/#comment124959>

    StringBuilder instead? Would be more apparent when we have (more) bugs like the one where we missed a slash in a hand-concat'ed path



server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java
<https://reviews.apache.org/r/32224/#comment124961>

    It's unlikely to be hit, but a finally would be nice here too.



server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java
<https://reviews.apache.org/r/32224/#comment124960>

    Need finally to close these.



server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java
<https://reviews.apache.org/r/32224/#comment124962>

    Need finally



server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java
<https://reviews.apache.org/r/32224/#comment124964>

    A few sets of unnecessary parens



server/gc/src/main/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferences.java
<https://reviews.apache.org/r/32224/#comment124966>

    Get rid of these or move them to trace



server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java
<https://reviews.apache.org/r/32224/#comment124970>

    I think this would be deserving a log message in the negative. Would help to know that the realized its state its up to date.



server/tserver/src/main/java/org/apache/accumulo/server/GarbageCollectionLogger.java
<https://reviews.apache.org/r/32224/#comment124971>

    This looks like it should be done in its own commit



server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
<https://reviews.apache.org/r/32224/#comment124972>

    Would help to be more clear as to why this is a warning. A user/admin wouldn't be able to parse what the msg means.



server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
<https://reviews.apache.org/r/32224/#comment124973>

    This block seems a bit dubious. Why do all calls to this method for non-root/meta loggers need to be synchronized? If the synchronization is intentional, some javadoc/comments would be greatly appreciated.



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

    Why is DfsLogger Comparable now? Isn't the log name still just a UUID?



server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java
<https://reviews.apache.org/r/32224/#comment124979>

    Don't you want to do this in the finally one more level "out" (and only if +oldName+ is non-null)?



server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java
<https://reviews.apache.org/r/32224/#comment124981>

    Better represented as (numAdded == 0 || numAdded == 1)


- Josh Elser


On March 19, 2015, 4:42 p.m., Eric Newton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32224/
> -----------------------------------------------------------
> 
> (Updated March 19, 2015, 4:42 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-3423
>     https://issues.apache.org/jira/browse/ACCUMULO-3423
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Faster WAL rollovers
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/client/impl/ReplicationOperationsImpl.java 6a5c74a 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java 2403915 
>   core/src/main/java/org/apache/accumulo/core/metadata/RootTable.java 24148b1 
>   core/src/main/java/org/apache/accumulo/core/metadata/schema/MetadataSchema.java 534dd7f 
>   core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java 25d0f32 
>   core/src/test/java/org/apache/accumulo/core/metadata/MetadataTableSchemaTest.java PRE-CREATION 
>   server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java fc26ca4 
>   server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java e5bdded 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataStateStore.java 7ee6f0c 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataTableScanner.java 9be5a67 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/TabletLocationState.java b24b562 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/TabletStateStore.java 5413e31 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/ZooTabletStateStore.java ab99396 
>   server/base/src/main/java/org/apache/accumulo/server/replication/StatusUtil.java 898e3d4 
>   server/base/src/main/java/org/apache/accumulo/server/util/ListVolumesUsed.java e90d1dd 
>   server/base/src/main/java/org/apache/accumulo/server/util/MasterMetadataUtil.java 4ca2d64 
>   server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java 10cd749 
>   server/base/src/main/java/org/apache/accumulo/server/util/ReplicationTableUtil.java af02a8d 
>   server/base/src/test/java/org/apache/accumulo/server/util/ReplicationTableUtilTest.java b1010c2 
>   server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java 1735c0d 
>   server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java c8d5cd6 
>   server/gc/src/main/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferences.java 3a32727 
>   server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogsTest.java 5801faa 
>   server/gc/src/test/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferencesTest.java 23db83a 
>   server/master/src/main/java/org/apache/accumulo/master/Master.java 3762f32 
>   server/master/src/main/java/org/apache/accumulo/master/MasterClientServiceHandler.java f73c236 
>   server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java d097d75 
>   server/master/src/main/java/org/apache/accumulo/master/replication/WorkMaker.java 8532e1b 
>   server/master/src/main/java/org/apache/accumulo/master/state/MergeStats.java 44f229e 
>   server/master/src/test/java/org/apache/accumulo/master/ReplicationOperationsImplTest.java a127dcd 
>   server/master/src/test/java/org/apache/accumulo/master/TestMergeState.java b0240f1 
>   server/master/src/test/java/org/apache/accumulo/master/state/RootTabletStateStoreTest.java abceae4 
>   server/tserver/src/main/findbugs/exclude-filter.xml 47dd1f5 
>   server/tserver/src/main/java/org/apache/accumulo/server/GarbageCollectionLogger.java 8f3785e 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletLevel.java PRE-CREATION 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java 662ee31 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java 5acf5eb 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/SortedLogRecovery.java c4d9fab 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java 711c497 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CommitSession.java b4814e4 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/DatafileManager.java 594d9c5 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java 6152500 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/TabletCommitter.java 4bc05a6 
>   test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java b429607 
>   test/src/test/java/org/apache/accumulo/proxy/ProxyDurabilityIT.java 404a8fd 
>   test/src/test/java/org/apache/accumulo/test/BadDeleteMarkersCreatedIT.java 25337b2 
>   test/src/test/java/org/apache/accumulo/test/BalanceIT.java f793925 
>   test/src/test/java/org/apache/accumulo/test/CleanWalIT.java f553be8 
>   test/src/test/java/org/apache/accumulo/test/ConditionalWriterIT.java bd00f02 
>   test/src/test/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java b78a311 
>   test/src/test/java/org/apache/accumulo/test/NoMutationRecoveryIT.java 6a9975c 
>   test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 56a6a70 
>   test/src/test/java/org/apache/accumulo/test/functional/WALSunnyDayIT.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/functional/WatchTheWatchCountIT.java bd0555b 
>   test/src/test/java/org/apache/accumulo/test/performance/RollWALPerformanceIT.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/replication/GarbageCollectorCommunicatesWithTServersIT.java 5b89d9c 
>   test/src/test/java/org/apache/accumulo/test/replication/MultiInstanceReplicationIT.java 9dec16e 
>   test/src/test/java/org/apache/accumulo/test/replication/ReplicationIT.java 54348db 
> 
> Diff: https://reviews.apache.org/r/32224/diff/
> 
> 
> Testing
> -------
> 
> Ran all tests, except RandomWalk.
> 
> 
> Thanks,
> 
> Eric Newton
> 
>


Re: Review Request 32224: ACCUMULO-3423

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


Finished reviewing this patch

- kturner


On March 19, 2015, 4:42 p.m., Eric Newton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32224/
> -----------------------------------------------------------
> 
> (Updated March 19, 2015, 4:42 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-3423
>     https://issues.apache.org/jira/browse/ACCUMULO-3423
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Faster WAL rollovers
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/client/impl/ReplicationOperationsImpl.java 6a5c74a 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java 2403915 
>   core/src/main/java/org/apache/accumulo/core/metadata/RootTable.java 24148b1 
>   core/src/main/java/org/apache/accumulo/core/metadata/schema/MetadataSchema.java 534dd7f 
>   core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java 25d0f32 
>   core/src/test/java/org/apache/accumulo/core/metadata/MetadataTableSchemaTest.java PRE-CREATION 
>   server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java fc26ca4 
>   server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java e5bdded 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataStateStore.java 7ee6f0c 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataTableScanner.java 9be5a67 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/TabletLocationState.java b24b562 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/TabletStateStore.java 5413e31 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/ZooTabletStateStore.java ab99396 
>   server/base/src/main/java/org/apache/accumulo/server/replication/StatusUtil.java 898e3d4 
>   server/base/src/main/java/org/apache/accumulo/server/util/ListVolumesUsed.java e90d1dd 
>   server/base/src/main/java/org/apache/accumulo/server/util/MasterMetadataUtil.java 4ca2d64 
>   server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java 10cd749 
>   server/base/src/main/java/org/apache/accumulo/server/util/ReplicationTableUtil.java af02a8d 
>   server/base/src/test/java/org/apache/accumulo/server/util/ReplicationTableUtilTest.java b1010c2 
>   server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java 1735c0d 
>   server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java c8d5cd6 
>   server/gc/src/main/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferences.java 3a32727 
>   server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogsTest.java 5801faa 
>   server/gc/src/test/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferencesTest.java 23db83a 
>   server/master/src/main/java/org/apache/accumulo/master/Master.java 3762f32 
>   server/master/src/main/java/org/apache/accumulo/master/MasterClientServiceHandler.java f73c236 
>   server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java d097d75 
>   server/master/src/main/java/org/apache/accumulo/master/replication/WorkMaker.java 8532e1b 
>   server/master/src/main/java/org/apache/accumulo/master/state/MergeStats.java 44f229e 
>   server/master/src/test/java/org/apache/accumulo/master/ReplicationOperationsImplTest.java a127dcd 
>   server/master/src/test/java/org/apache/accumulo/master/TestMergeState.java b0240f1 
>   server/master/src/test/java/org/apache/accumulo/master/state/RootTabletStateStoreTest.java abceae4 
>   server/tserver/src/main/findbugs/exclude-filter.xml 47dd1f5 
>   server/tserver/src/main/java/org/apache/accumulo/server/GarbageCollectionLogger.java 8f3785e 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletLevel.java PRE-CREATION 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java 662ee31 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java 5acf5eb 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/SortedLogRecovery.java c4d9fab 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java 711c497 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CommitSession.java b4814e4 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/DatafileManager.java 594d9c5 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java 6152500 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/TabletCommitter.java 4bc05a6 
>   test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java b429607 
>   test/src/test/java/org/apache/accumulo/proxy/ProxyDurabilityIT.java 404a8fd 
>   test/src/test/java/org/apache/accumulo/test/BadDeleteMarkersCreatedIT.java 25337b2 
>   test/src/test/java/org/apache/accumulo/test/BalanceIT.java f793925 
>   test/src/test/java/org/apache/accumulo/test/CleanWalIT.java f553be8 
>   test/src/test/java/org/apache/accumulo/test/ConditionalWriterIT.java bd00f02 
>   test/src/test/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java b78a311 
>   test/src/test/java/org/apache/accumulo/test/NoMutationRecoveryIT.java 6a9975c 
>   test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 56a6a70 
>   test/src/test/java/org/apache/accumulo/test/functional/WALSunnyDayIT.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/functional/WatchTheWatchCountIT.java bd0555b 
>   test/src/test/java/org/apache/accumulo/test/performance/RollWALPerformanceIT.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/replication/GarbageCollectorCommunicatesWithTServersIT.java 5b89d9c 
>   test/src/test/java/org/apache/accumulo/test/replication/MultiInstanceReplicationIT.java 9dec16e 
>   test/src/test/java/org/apache/accumulo/test/replication/ReplicationIT.java 54348db 
> 
> Diff: https://reviews.apache.org/r/32224/diff/
> 
> 
> Testing
> -------
> 
> Ran all tests, except RandomWalk.
> 
> 
> Thanks,
> 
> Eric Newton
> 
>


Re: Review Request 32224: ACCUMULO-3423

Posted by Eric Newton <er...@gmail.com>.

> On March 27, 2015, 3:43 p.m., kturner wrote:
> > core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java, line 125
> > <https://reviews.apache.org/r/32224/diff/1/?file=899476#file899476line125>
> >
> >     should there be a sanity check on the length of `parts[]`?  Maybe the construction should do the sanity check

Added a unit test for LogEntry, and check the format of the string a little more carefully.


> On March 27, 2015, 3:43 p.m., kturner wrote:
> > server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java, line 1783
> > <https://reviews.apache.org/r/32224/diff/1/?file=899507#file899507line1783>
> >
> >     why not just remove this thrift call?

[~elserj] wanted it in there.


> On March 27, 2015, 3:43 p.m., kturner wrote:
> > server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java, line 292
> > <https://reviews.apache.org/r/32224/diff/1/?file=899498#file899498line292>
> >
> >     The code to merge tablets had checks to ensure it would not merge tablets with walogs.  Need to ensure the updates to add walogs to a tablet are always made before those checks.   Looking at the code, it seems like this may be the case but I am not completely sure.   Maybe some test for this situation?  Maybe shard and bulk random walk with agitation is enought to test this situation.  Do you think ITs are needed?

The only way to get unassigned by the master is to get the list of WALogs added at the same time.


> On March 27, 2015, 3:43 p.m., kturner wrote:
> > server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java, line 210
> > <https://reviews.apache.org/r/32224/diff/1/?file=899510#file899510line210>
> >
> >     I think the following code to create a log could be removed with the SynchronousQueue... just always take from the syncronous queue

Good catch.  That improved the code a great deal.


> On March 27, 2015, 3:43 p.m., kturner wrote:
> > server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java, line 243
> > <https://reviews.apache.org/r/32224/diff/1/?file=899510#file899510line243>
> >
> >     why not just move check up to beginning and bail if there is already a next log?

Used the SynchronousQueue as suggested.


> On March 27, 2015, 3:43 p.m., kturner wrote:
> > server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java, line 240
> > <https://reviews.apache.org/r/32224/diff/1/?file=899510#file899510line240>
> >
> >     Could iterate through tablets and find levels present, then pass those levels to `addLoggersToMetadata()`.  This would avoid synchronizing on each tablet.

Good idea.


> On March 27, 2015, 3:43 p.m., kturner wrote:
> > server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java, line 245
> > <https://reviews.apache.org/r/32224/diff/1/?file=899510#file899510line245>
> >
> >     Is this expected to occur often?  Should this unused log be marked unused in  metadata table?

Code has been restructured using the SynchronousQueue, and this warning will never occure.


- Eric


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


On March 19, 2015, 4:42 p.m., Eric Newton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32224/
> -----------------------------------------------------------
> 
> (Updated March 19, 2015, 4:42 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-3423
>     https://issues.apache.org/jira/browse/ACCUMULO-3423
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Faster WAL rollovers
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/client/impl/ReplicationOperationsImpl.java 6a5c74a 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java 2403915 
>   core/src/main/java/org/apache/accumulo/core/metadata/RootTable.java 24148b1 
>   core/src/main/java/org/apache/accumulo/core/metadata/schema/MetadataSchema.java 534dd7f 
>   core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java 25d0f32 
>   core/src/test/java/org/apache/accumulo/core/metadata/MetadataTableSchemaTest.java PRE-CREATION 
>   server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java fc26ca4 
>   server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java e5bdded 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataStateStore.java 7ee6f0c 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataTableScanner.java 9be5a67 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/TabletLocationState.java b24b562 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/TabletStateStore.java 5413e31 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/ZooTabletStateStore.java ab99396 
>   server/base/src/main/java/org/apache/accumulo/server/replication/StatusUtil.java 898e3d4 
>   server/base/src/main/java/org/apache/accumulo/server/util/ListVolumesUsed.java e90d1dd 
>   server/base/src/main/java/org/apache/accumulo/server/util/MasterMetadataUtil.java 4ca2d64 
>   server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java 10cd749 
>   server/base/src/main/java/org/apache/accumulo/server/util/ReplicationTableUtil.java af02a8d 
>   server/base/src/test/java/org/apache/accumulo/server/util/ReplicationTableUtilTest.java b1010c2 
>   server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java 1735c0d 
>   server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java c8d5cd6 
>   server/gc/src/main/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferences.java 3a32727 
>   server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogsTest.java 5801faa 
>   server/gc/src/test/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferencesTest.java 23db83a 
>   server/master/src/main/java/org/apache/accumulo/master/Master.java 3762f32 
>   server/master/src/main/java/org/apache/accumulo/master/MasterClientServiceHandler.java f73c236 
>   server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java d097d75 
>   server/master/src/main/java/org/apache/accumulo/master/replication/WorkMaker.java 8532e1b 
>   server/master/src/main/java/org/apache/accumulo/master/state/MergeStats.java 44f229e 
>   server/master/src/test/java/org/apache/accumulo/master/ReplicationOperationsImplTest.java a127dcd 
>   server/master/src/test/java/org/apache/accumulo/master/TestMergeState.java b0240f1 
>   server/master/src/test/java/org/apache/accumulo/master/state/RootTabletStateStoreTest.java abceae4 
>   server/tserver/src/main/findbugs/exclude-filter.xml 47dd1f5 
>   server/tserver/src/main/java/org/apache/accumulo/server/GarbageCollectionLogger.java 8f3785e 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletLevel.java PRE-CREATION 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java 662ee31 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java 5acf5eb 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/SortedLogRecovery.java c4d9fab 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java 711c497 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CommitSession.java b4814e4 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/DatafileManager.java 594d9c5 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java 6152500 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/TabletCommitter.java 4bc05a6 
>   test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java b429607 
>   test/src/test/java/org/apache/accumulo/proxy/ProxyDurabilityIT.java 404a8fd 
>   test/src/test/java/org/apache/accumulo/test/BadDeleteMarkersCreatedIT.java 25337b2 
>   test/src/test/java/org/apache/accumulo/test/BalanceIT.java f793925 
>   test/src/test/java/org/apache/accumulo/test/CleanWalIT.java f553be8 
>   test/src/test/java/org/apache/accumulo/test/ConditionalWriterIT.java bd00f02 
>   test/src/test/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java b78a311 
>   test/src/test/java/org/apache/accumulo/test/NoMutationRecoveryIT.java 6a9975c 
>   test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 56a6a70 
>   test/src/test/java/org/apache/accumulo/test/functional/WALSunnyDayIT.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/functional/WatchTheWatchCountIT.java bd0555b 
>   test/src/test/java/org/apache/accumulo/test/performance/RollWALPerformanceIT.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/replication/GarbageCollectorCommunicatesWithTServersIT.java 5b89d9c 
>   test/src/test/java/org/apache/accumulo/test/replication/MultiInstanceReplicationIT.java 9dec16e 
>   test/src/test/java/org/apache/accumulo/test/replication/ReplicationIT.java 54348db 
> 
> Diff: https://reviews.apache.org/r/32224/diff/
> 
> 
> Testing
> -------
> 
> Ran all tests, except RandomWalk.
> 
> 
> Thanks,
> 
> Eric Newton
> 
>


Re: Review Request 32224: ACCUMULO-3423

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


Still reviewing... just have page 3 and CommitSession left


core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java
<https://reviews.apache.org/r/32224/#comment126448>

    should there be a sanity check on the length of `parts[]`?  Maybe the construction should do the sanity check



server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java
<https://reviews.apache.org/r/32224/#comment126438>

    The code to merge tablets had checks to ensure it would not merge tablets with walogs.  Need to ensure the updates to add walogs to a tablet are always made before those checks.   Looking at the code, it seems like this may be the case but I am not completely sure.   Maybe some test for this situation?  Maybe shard and bulk random walk with agitation is enought to test this situation.  Do you think ITs are needed?



server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
<https://reviews.apache.org/r/32224/#comment126441>

    why not just remove this thrift call?



server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java
<https://reviews.apache.org/r/32224/#comment126451>

    I think the following code to create a log could be removed with the SynchronousQueue... just always take from the syncronous queue



server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java
<https://reviews.apache.org/r/32224/#comment126443>

    <p>reading the javadoc for this, seems like it does not consider # queued... only #executing</p>
    
    Thinking that background log create could be simplified a lot with a SynchronousQueue.  Could remove code in other places that creates walogs.
    
        class LogCreator extends Runnable {
          SynchronousQueue logQueue;
           
          public void run(){
            while(true){
              //create new log and add to metadata
              //this is the only place in the code
              //that creates new walogs, no need to
              //do it anywhere else
              DfsLogger walog = createNewLog();
              
              //wait until another thread takes the log
              logQueue.put(walog);
            }    
          }
        }



server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java
<https://reviews.apache.org/r/32224/#comment126446>

    Could iterate through tablets and find levels present, then pass those levels to `addLoggersToMetadata()`.  This would avoid synchronizing on each tablet.



server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java
<https://reviews.apache.org/r/32224/#comment126444>

    why not just move check up to beginning and bail if there is already a next log?



server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java
<https://reviews.apache.org/r/32224/#comment126442>

    Is this expected to occur often?  Should this unused log be marked unused in  metadata table?


- kturner


On March 19, 2015, 4:42 p.m., Eric Newton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32224/
> -----------------------------------------------------------
> 
> (Updated March 19, 2015, 4:42 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-3423
>     https://issues.apache.org/jira/browse/ACCUMULO-3423
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Faster WAL rollovers
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/client/impl/ReplicationOperationsImpl.java 6a5c74a 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java 2403915 
>   core/src/main/java/org/apache/accumulo/core/metadata/RootTable.java 24148b1 
>   core/src/main/java/org/apache/accumulo/core/metadata/schema/MetadataSchema.java 534dd7f 
>   core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java 25d0f32 
>   core/src/test/java/org/apache/accumulo/core/metadata/MetadataTableSchemaTest.java PRE-CREATION 
>   server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java fc26ca4 
>   server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java e5bdded 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataStateStore.java 7ee6f0c 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataTableScanner.java 9be5a67 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/TabletLocationState.java b24b562 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/TabletStateStore.java 5413e31 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/ZooTabletStateStore.java ab99396 
>   server/base/src/main/java/org/apache/accumulo/server/replication/StatusUtil.java 898e3d4 
>   server/base/src/main/java/org/apache/accumulo/server/util/ListVolumesUsed.java e90d1dd 
>   server/base/src/main/java/org/apache/accumulo/server/util/MasterMetadataUtil.java 4ca2d64 
>   server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java 10cd749 
>   server/base/src/main/java/org/apache/accumulo/server/util/ReplicationTableUtil.java af02a8d 
>   server/base/src/test/java/org/apache/accumulo/server/util/ReplicationTableUtilTest.java b1010c2 
>   server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java 1735c0d 
>   server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java c8d5cd6 
>   server/gc/src/main/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferences.java 3a32727 
>   server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogsTest.java 5801faa 
>   server/gc/src/test/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferencesTest.java 23db83a 
>   server/master/src/main/java/org/apache/accumulo/master/Master.java 3762f32 
>   server/master/src/main/java/org/apache/accumulo/master/MasterClientServiceHandler.java f73c236 
>   server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java d097d75 
>   server/master/src/main/java/org/apache/accumulo/master/replication/WorkMaker.java 8532e1b 
>   server/master/src/main/java/org/apache/accumulo/master/state/MergeStats.java 44f229e 
>   server/master/src/test/java/org/apache/accumulo/master/ReplicationOperationsImplTest.java a127dcd 
>   server/master/src/test/java/org/apache/accumulo/master/TestMergeState.java b0240f1 
>   server/master/src/test/java/org/apache/accumulo/master/state/RootTabletStateStoreTest.java abceae4 
>   server/tserver/src/main/findbugs/exclude-filter.xml 47dd1f5 
>   server/tserver/src/main/java/org/apache/accumulo/server/GarbageCollectionLogger.java 8f3785e 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletLevel.java PRE-CREATION 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java 662ee31 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java 5acf5eb 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/SortedLogRecovery.java c4d9fab 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java 711c497 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CommitSession.java b4814e4 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/DatafileManager.java 594d9c5 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java 6152500 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/TabletCommitter.java 4bc05a6 
>   test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java b429607 
>   test/src/test/java/org/apache/accumulo/proxy/ProxyDurabilityIT.java 404a8fd 
>   test/src/test/java/org/apache/accumulo/test/BadDeleteMarkersCreatedIT.java 25337b2 
>   test/src/test/java/org/apache/accumulo/test/BalanceIT.java f793925 
>   test/src/test/java/org/apache/accumulo/test/CleanWalIT.java f553be8 
>   test/src/test/java/org/apache/accumulo/test/ConditionalWriterIT.java bd00f02 
>   test/src/test/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java b78a311 
>   test/src/test/java/org/apache/accumulo/test/NoMutationRecoveryIT.java 6a9975c 
>   test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 56a6a70 
>   test/src/test/java/org/apache/accumulo/test/functional/WALSunnyDayIT.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/functional/WatchTheWatchCountIT.java bd0555b 
>   test/src/test/java/org/apache/accumulo/test/performance/RollWALPerformanceIT.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/replication/GarbageCollectorCommunicatesWithTServersIT.java 5b89d9c 
>   test/src/test/java/org/apache/accumulo/test/replication/MultiInstanceReplicationIT.java 9dec16e 
>   test/src/test/java/org/apache/accumulo/test/replication/ReplicationIT.java 54348db 
> 
> Diff: https://reviews.apache.org/r/32224/diff/
> 
> 
> Testing
> -------
> 
> Ran all tests, except RandomWalk.
> 
> 
> Thanks,
> 
> Eric Newton
> 
>


Re: Review Request 32224: ACCUMULO-3423

Posted by Eric Newton <er...@gmail.com>.

> On March 23, 2015, 5:39 p.m., kturner wrote:
> > server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java, line 1154
> > <https://reviews.apache.org/r/32224/diff/1/?file=899488#file899488line1154>
> >
> >     Why does this write to root and meta?

There could be a marker at the meta and/or root level, mark them both as unused. Comment added.


> On March 23, 2015, 5:39 p.m., kturner wrote:
> > server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java, line 1199
> > <https://reviews.apache.org/r/32224/diff/1/?file=899488#file899488line1199>
> >
> >     What prevents unused logs from being fetched?

Good catch. I've fixed the code.


> On March 23, 2015, 5:39 p.m., kturner wrote:
> > server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java, line 232
> > <https://reviews.apache.org/r/32224/diff/1/?file=899498#file899498line232>
> >
> >     Is this code still needed?  Would it still be used for upgrade case?

I tried to make ACCUMULO-3423 as conservative as possible (yes, really!). To that end, log entries are put onto tablets for recovery, so all the trickiness of recovery could stay the same.


> On March 23, 2015, 5:39 p.m., kturner wrote:
> > server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java, line 273
> > <https://reviews.apache.org/r/32224/diff/1/?file=899498#file899498line273>
> >
> >     Would you expect this method to be called if the logs had already been marked as unused?  It seems like it should not, but I am not 100% sure.  Thinking that if this is an unexpected case that a sanity check might be worthwhile.

The metadata update to unassign a tablet creates the log references in the same mutation.  It should never be the case that the tablet is ASSIGNED to dead server and not need to fetch the logs.


- Eric


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


On March 19, 2015, 4:42 p.m., Eric Newton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32224/
> -----------------------------------------------------------
> 
> (Updated March 19, 2015, 4:42 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-3423
>     https://issues.apache.org/jira/browse/ACCUMULO-3423
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Faster WAL rollovers
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/client/impl/ReplicationOperationsImpl.java 6a5c74a 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java 2403915 
>   core/src/main/java/org/apache/accumulo/core/metadata/RootTable.java 24148b1 
>   core/src/main/java/org/apache/accumulo/core/metadata/schema/MetadataSchema.java 534dd7f 
>   core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java 25d0f32 
>   core/src/test/java/org/apache/accumulo/core/metadata/MetadataTableSchemaTest.java PRE-CREATION 
>   server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java fc26ca4 
>   server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java e5bdded 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataStateStore.java 7ee6f0c 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataTableScanner.java 9be5a67 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/TabletLocationState.java b24b562 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/TabletStateStore.java 5413e31 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/ZooTabletStateStore.java ab99396 
>   server/base/src/main/java/org/apache/accumulo/server/replication/StatusUtil.java 898e3d4 
>   server/base/src/main/java/org/apache/accumulo/server/util/ListVolumesUsed.java e90d1dd 
>   server/base/src/main/java/org/apache/accumulo/server/util/MasterMetadataUtil.java 4ca2d64 
>   server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java 10cd749 
>   server/base/src/main/java/org/apache/accumulo/server/util/ReplicationTableUtil.java af02a8d 
>   server/base/src/test/java/org/apache/accumulo/server/util/ReplicationTableUtilTest.java b1010c2 
>   server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java 1735c0d 
>   server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java c8d5cd6 
>   server/gc/src/main/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferences.java 3a32727 
>   server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogsTest.java 5801faa 
>   server/gc/src/test/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferencesTest.java 23db83a 
>   server/master/src/main/java/org/apache/accumulo/master/Master.java 3762f32 
>   server/master/src/main/java/org/apache/accumulo/master/MasterClientServiceHandler.java f73c236 
>   server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java d097d75 
>   server/master/src/main/java/org/apache/accumulo/master/replication/WorkMaker.java 8532e1b 
>   server/master/src/main/java/org/apache/accumulo/master/state/MergeStats.java 44f229e 
>   server/master/src/test/java/org/apache/accumulo/master/ReplicationOperationsImplTest.java a127dcd 
>   server/master/src/test/java/org/apache/accumulo/master/TestMergeState.java b0240f1 
>   server/master/src/test/java/org/apache/accumulo/master/state/RootTabletStateStoreTest.java abceae4 
>   server/tserver/src/main/findbugs/exclude-filter.xml 47dd1f5 
>   server/tserver/src/main/java/org/apache/accumulo/server/GarbageCollectionLogger.java 8f3785e 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletLevel.java PRE-CREATION 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java 662ee31 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java 5acf5eb 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/SortedLogRecovery.java c4d9fab 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java 711c497 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CommitSession.java b4814e4 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/DatafileManager.java 594d9c5 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java 6152500 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/TabletCommitter.java 4bc05a6 
>   test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java b429607 
>   test/src/test/java/org/apache/accumulo/proxy/ProxyDurabilityIT.java 404a8fd 
>   test/src/test/java/org/apache/accumulo/test/BadDeleteMarkersCreatedIT.java 25337b2 
>   test/src/test/java/org/apache/accumulo/test/BalanceIT.java f793925 
>   test/src/test/java/org/apache/accumulo/test/CleanWalIT.java f553be8 
>   test/src/test/java/org/apache/accumulo/test/ConditionalWriterIT.java bd00f02 
>   test/src/test/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java b78a311 
>   test/src/test/java/org/apache/accumulo/test/NoMutationRecoveryIT.java 6a9975c 
>   test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 56a6a70 
>   test/src/test/java/org/apache/accumulo/test/functional/WALSunnyDayIT.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/functional/WatchTheWatchCountIT.java bd0555b 
>   test/src/test/java/org/apache/accumulo/test/performance/RollWALPerformanceIT.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/replication/GarbageCollectorCommunicatesWithTServersIT.java 5b89d9c 
>   test/src/test/java/org/apache/accumulo/test/replication/MultiInstanceReplicationIT.java 9dec16e 
>   test/src/test/java/org/apache/accumulo/test/replication/ReplicationIT.java 54348db 
> 
> Diff: https://reviews.apache.org/r/32224/diff/
> 
> 
> Testing
> -------
> 
> Ran all tests, except RandomWalk.
> 
> 
> Thanks,
> 
> Eric Newton
> 
>


Re: Review Request 32224: ACCUMULO-3423

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


still reviewing....


server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java
<https://reviews.apache.org/r/32224/#comment125480>

    Why does this write to root and meta?



server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java
<https://reviews.apache.org/r/32224/#comment125481>

    What prevents unused logs from being fetched?



server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java
<https://reviews.apache.org/r/32224/#comment125464>

    Is this code still needed?  Would it still be used for upgrade case?



server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java
<https://reviews.apache.org/r/32224/#comment125469>

    Would you expect this method to be called if the logs had already been marked as unused?  It seems like it should not, but I am not 100% sure.  Thinking that if this is an unexpected case that a sanity check might be worthwhile.



server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
<https://reviews.apache.org/r/32224/#comment125473>

    Synchronizing on enum, enum is at classloader level.  Would be nice to sync on something private to tserver.


- kturner


On March 19, 2015, 4:42 p.m., Eric Newton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32224/
> -----------------------------------------------------------
> 
> (Updated March 19, 2015, 4:42 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-3423
>     https://issues.apache.org/jira/browse/ACCUMULO-3423
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Faster WAL rollovers
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/client/impl/ReplicationOperationsImpl.java 6a5c74a 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java 2403915 
>   core/src/main/java/org/apache/accumulo/core/metadata/RootTable.java 24148b1 
>   core/src/main/java/org/apache/accumulo/core/metadata/schema/MetadataSchema.java 534dd7f 
>   core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java 25d0f32 
>   core/src/test/java/org/apache/accumulo/core/metadata/MetadataTableSchemaTest.java PRE-CREATION 
>   server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java fc26ca4 
>   server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java e5bdded 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataStateStore.java 7ee6f0c 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataTableScanner.java 9be5a67 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/TabletLocationState.java b24b562 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/TabletStateStore.java 5413e31 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/ZooTabletStateStore.java ab99396 
>   server/base/src/main/java/org/apache/accumulo/server/replication/StatusUtil.java 898e3d4 
>   server/base/src/main/java/org/apache/accumulo/server/util/ListVolumesUsed.java e90d1dd 
>   server/base/src/main/java/org/apache/accumulo/server/util/MasterMetadataUtil.java 4ca2d64 
>   server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java 10cd749 
>   server/base/src/main/java/org/apache/accumulo/server/util/ReplicationTableUtil.java af02a8d 
>   server/base/src/test/java/org/apache/accumulo/server/util/ReplicationTableUtilTest.java b1010c2 
>   server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java 1735c0d 
>   server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java c8d5cd6 
>   server/gc/src/main/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferences.java 3a32727 
>   server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogsTest.java 5801faa 
>   server/gc/src/test/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferencesTest.java 23db83a 
>   server/master/src/main/java/org/apache/accumulo/master/Master.java 3762f32 
>   server/master/src/main/java/org/apache/accumulo/master/MasterClientServiceHandler.java f73c236 
>   server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java d097d75 
>   server/master/src/main/java/org/apache/accumulo/master/replication/WorkMaker.java 8532e1b 
>   server/master/src/main/java/org/apache/accumulo/master/state/MergeStats.java 44f229e 
>   server/master/src/test/java/org/apache/accumulo/master/ReplicationOperationsImplTest.java a127dcd 
>   server/master/src/test/java/org/apache/accumulo/master/TestMergeState.java b0240f1 
>   server/master/src/test/java/org/apache/accumulo/master/state/RootTabletStateStoreTest.java abceae4 
>   server/tserver/src/main/findbugs/exclude-filter.xml 47dd1f5 
>   server/tserver/src/main/java/org/apache/accumulo/server/GarbageCollectionLogger.java 8f3785e 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletLevel.java PRE-CREATION 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java 662ee31 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java 5acf5eb 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/SortedLogRecovery.java c4d9fab 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java 711c497 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CommitSession.java b4814e4 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/DatafileManager.java 594d9c5 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java 6152500 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/TabletCommitter.java 4bc05a6 
>   test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java b429607 
>   test/src/test/java/org/apache/accumulo/proxy/ProxyDurabilityIT.java 404a8fd 
>   test/src/test/java/org/apache/accumulo/test/BadDeleteMarkersCreatedIT.java 25337b2 
>   test/src/test/java/org/apache/accumulo/test/BalanceIT.java f793925 
>   test/src/test/java/org/apache/accumulo/test/CleanWalIT.java f553be8 
>   test/src/test/java/org/apache/accumulo/test/ConditionalWriterIT.java bd00f02 
>   test/src/test/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java b78a311 
>   test/src/test/java/org/apache/accumulo/test/NoMutationRecoveryIT.java 6a9975c 
>   test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 56a6a70 
>   test/src/test/java/org/apache/accumulo/test/functional/WALSunnyDayIT.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/functional/WatchTheWatchCountIT.java bd0555b 
>   test/src/test/java/org/apache/accumulo/test/performance/RollWALPerformanceIT.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/replication/GarbageCollectorCommunicatesWithTServersIT.java 5b89d9c 
>   test/src/test/java/org/apache/accumulo/test/replication/MultiInstanceReplicationIT.java 9dec16e 
>   test/src/test/java/org/apache/accumulo/test/replication/ReplicationIT.java 54348db 
> 
> Diff: https://reviews.apache.org/r/32224/diff/
> 
> 
> Testing
> -------
> 
> Ran all tests, except RandomWalk.
> 
> 
> Thanks,
> 
> Eric Newton
> 
>


Re: Review Request 32224: ACCUMULO-3423

Posted by Eric Newton <er...@gmail.com>.

> On March 20, 2015, 7:11 p.m., kturner wrote:
> > server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java, line 137
> > <https://reviews.apache.org/r/32224/diff/1/?file=899478#file899478line137>
> >
> >     IT that test volume replacement and log recovery, would be very nice if it does not exists.  This would test if volume replacement works w/ these new ~wal+ metadata table entries.

Added a test to VolumeIT


- Eric


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


On March 19, 2015, 4:42 p.m., Eric Newton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32224/
> -----------------------------------------------------------
> 
> (Updated March 19, 2015, 4:42 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-3423
>     https://issues.apache.org/jira/browse/ACCUMULO-3423
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Faster WAL rollovers
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/client/impl/ReplicationOperationsImpl.java 6a5c74a 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java 2403915 
>   core/src/main/java/org/apache/accumulo/core/metadata/RootTable.java 24148b1 
>   core/src/main/java/org/apache/accumulo/core/metadata/schema/MetadataSchema.java 534dd7f 
>   core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java 25d0f32 
>   core/src/test/java/org/apache/accumulo/core/metadata/MetadataTableSchemaTest.java PRE-CREATION 
>   server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java fc26ca4 
>   server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java e5bdded 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataStateStore.java 7ee6f0c 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataTableScanner.java 9be5a67 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/TabletLocationState.java b24b562 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/TabletStateStore.java 5413e31 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/ZooTabletStateStore.java ab99396 
>   server/base/src/main/java/org/apache/accumulo/server/replication/StatusUtil.java 898e3d4 
>   server/base/src/main/java/org/apache/accumulo/server/util/ListVolumesUsed.java e90d1dd 
>   server/base/src/main/java/org/apache/accumulo/server/util/MasterMetadataUtil.java 4ca2d64 
>   server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java 10cd749 
>   server/base/src/main/java/org/apache/accumulo/server/util/ReplicationTableUtil.java af02a8d 
>   server/base/src/test/java/org/apache/accumulo/server/util/ReplicationTableUtilTest.java b1010c2 
>   server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java 1735c0d 
>   server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java c8d5cd6 
>   server/gc/src/main/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferences.java 3a32727 
>   server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogsTest.java 5801faa 
>   server/gc/src/test/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferencesTest.java 23db83a 
>   server/master/src/main/java/org/apache/accumulo/master/Master.java 3762f32 
>   server/master/src/main/java/org/apache/accumulo/master/MasterClientServiceHandler.java f73c236 
>   server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java d097d75 
>   server/master/src/main/java/org/apache/accumulo/master/replication/WorkMaker.java 8532e1b 
>   server/master/src/main/java/org/apache/accumulo/master/state/MergeStats.java 44f229e 
>   server/master/src/test/java/org/apache/accumulo/master/ReplicationOperationsImplTest.java a127dcd 
>   server/master/src/test/java/org/apache/accumulo/master/TestMergeState.java b0240f1 
>   server/master/src/test/java/org/apache/accumulo/master/state/RootTabletStateStoreTest.java abceae4 
>   server/tserver/src/main/findbugs/exclude-filter.xml 47dd1f5 
>   server/tserver/src/main/java/org/apache/accumulo/server/GarbageCollectionLogger.java 8f3785e 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletLevel.java PRE-CREATION 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java 662ee31 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java 5acf5eb 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/SortedLogRecovery.java c4d9fab 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java 711c497 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CommitSession.java b4814e4 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/DatafileManager.java 594d9c5 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java 6152500 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/TabletCommitter.java 4bc05a6 
>   test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java b429607 
>   test/src/test/java/org/apache/accumulo/proxy/ProxyDurabilityIT.java 404a8fd 
>   test/src/test/java/org/apache/accumulo/test/BadDeleteMarkersCreatedIT.java 25337b2 
>   test/src/test/java/org/apache/accumulo/test/BalanceIT.java f793925 
>   test/src/test/java/org/apache/accumulo/test/CleanWalIT.java f553be8 
>   test/src/test/java/org/apache/accumulo/test/ConditionalWriterIT.java bd00f02 
>   test/src/test/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java b78a311 
>   test/src/test/java/org/apache/accumulo/test/NoMutationRecoveryIT.java 6a9975c 
>   test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 56a6a70 
>   test/src/test/java/org/apache/accumulo/test/functional/WALSunnyDayIT.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/functional/WatchTheWatchCountIT.java bd0555b 
>   test/src/test/java/org/apache/accumulo/test/performance/RollWALPerformanceIT.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/replication/GarbageCollectorCommunicatesWithTServersIT.java 5b89d9c 
>   test/src/test/java/org/apache/accumulo/test/replication/MultiInstanceReplicationIT.java 9dec16e 
>   test/src/test/java/org/apache/accumulo/test/replication/ReplicationIT.java 54348db 
> 
> Diff: https://reviews.apache.org/r/32224/diff/
> 
> 
> Testing
> -------
> 
> Ran all tests, except RandomWalk.
> 
> 
> Thanks,
> 
> Eric Newton
> 
>


Re: Review Request 32224: ACCUMULO-3423

Posted by Eric Newton <er...@gmail.com>.

> On March 20, 2015, 7:11 p.m., kturner wrote:
> > This change make the case where a tablet has some non-empty WALs with no data more likely (not sure how likely it would have been before).  For example a tablet could have WAL1 with data for it, WAL2 with no data for it, and WAL3 with data.   Seems like continuous ingest would not trigger this case.  Wonder if an IT for this case is worthwhile.
> > 
> > I am still reviewing.  Was getting nervous about RB losing my comments, so posting some comments I made so far.

Added an IT for this case, in particular. Also added tests to ensure that empty logs for dead tablet servers are marked unused and are deleted.


> On March 20, 2015, 7:11 p.m., kturner wrote:
> > server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataStateStore.java, line 139
> > <https://reviews.apache.org/r/32224/diff/1/?file=899480#file899480line139>
> >
> >     Seems like we could end with logs for multiple tservers in the following case.
> >     
> >       1. Tablet T1 is assigned to Tserver1
> >       2. Tserver1.I1 dies
> >       3. Master adds logs for Tserver1 to Tablet T1
> >       4. Master assigns Tablet T1 to Tserver2
> >       5. Before Tserver T2 loads and recovers Tablet T1, it dies
> >       6. Master adds logs for Tserver2 to Tablet T1
> >     
> >     In this case Tablet T1 has walog from two tservers.  Not sure how well recovery code would handle this.   Its seems like recovery code makes the assumption that walogs are from one tserver.

Modified the code so that logs are only written if the tablet was actually hosted.  This should prevent logs from being added before the old logs are recovered.


> On March 20, 2015, 7:11 p.m., kturner wrote:
> > server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java, line 293
> > <https://reviews.apache.org/r/32224/diff/1/?file=899498#file899498line293>
> >
> >     If no tablets are assigned to a dead tserver, will its logs be marked as unused?  Was wondering if this operation is idempotent.  If the master dies right after flushChanges, then it seems like no tablets would be assigned to the dead server.

Added a scan to the GC to remove logs without metadata reference for dead servers.


- Eric


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


On March 19, 2015, 4:42 p.m., Eric Newton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32224/
> -----------------------------------------------------------
> 
> (Updated March 19, 2015, 4:42 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-3423
>     https://issues.apache.org/jira/browse/ACCUMULO-3423
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Faster WAL rollovers
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/client/impl/ReplicationOperationsImpl.java 6a5c74a 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java 2403915 
>   core/src/main/java/org/apache/accumulo/core/metadata/RootTable.java 24148b1 
>   core/src/main/java/org/apache/accumulo/core/metadata/schema/MetadataSchema.java 534dd7f 
>   core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java 25d0f32 
>   core/src/test/java/org/apache/accumulo/core/metadata/MetadataTableSchemaTest.java PRE-CREATION 
>   server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java fc26ca4 
>   server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java e5bdded 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataStateStore.java 7ee6f0c 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataTableScanner.java 9be5a67 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/TabletLocationState.java b24b562 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/TabletStateStore.java 5413e31 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/ZooTabletStateStore.java ab99396 
>   server/base/src/main/java/org/apache/accumulo/server/replication/StatusUtil.java 898e3d4 
>   server/base/src/main/java/org/apache/accumulo/server/util/ListVolumesUsed.java e90d1dd 
>   server/base/src/main/java/org/apache/accumulo/server/util/MasterMetadataUtil.java 4ca2d64 
>   server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java 10cd749 
>   server/base/src/main/java/org/apache/accumulo/server/util/ReplicationTableUtil.java af02a8d 
>   server/base/src/test/java/org/apache/accumulo/server/util/ReplicationTableUtilTest.java b1010c2 
>   server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java 1735c0d 
>   server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java c8d5cd6 
>   server/gc/src/main/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferences.java 3a32727 
>   server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogsTest.java 5801faa 
>   server/gc/src/test/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferencesTest.java 23db83a 
>   server/master/src/main/java/org/apache/accumulo/master/Master.java 3762f32 
>   server/master/src/main/java/org/apache/accumulo/master/MasterClientServiceHandler.java f73c236 
>   server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java d097d75 
>   server/master/src/main/java/org/apache/accumulo/master/replication/WorkMaker.java 8532e1b 
>   server/master/src/main/java/org/apache/accumulo/master/state/MergeStats.java 44f229e 
>   server/master/src/test/java/org/apache/accumulo/master/ReplicationOperationsImplTest.java a127dcd 
>   server/master/src/test/java/org/apache/accumulo/master/TestMergeState.java b0240f1 
>   server/master/src/test/java/org/apache/accumulo/master/state/RootTabletStateStoreTest.java abceae4 
>   server/tserver/src/main/findbugs/exclude-filter.xml 47dd1f5 
>   server/tserver/src/main/java/org/apache/accumulo/server/GarbageCollectionLogger.java 8f3785e 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletLevel.java PRE-CREATION 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java 662ee31 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java 5acf5eb 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/SortedLogRecovery.java c4d9fab 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java 711c497 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CommitSession.java b4814e4 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/DatafileManager.java 594d9c5 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java 6152500 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/TabletCommitter.java 4bc05a6 
>   test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java b429607 
>   test/src/test/java/org/apache/accumulo/proxy/ProxyDurabilityIT.java 404a8fd 
>   test/src/test/java/org/apache/accumulo/test/BadDeleteMarkersCreatedIT.java 25337b2 
>   test/src/test/java/org/apache/accumulo/test/BalanceIT.java f793925 
>   test/src/test/java/org/apache/accumulo/test/CleanWalIT.java f553be8 
>   test/src/test/java/org/apache/accumulo/test/ConditionalWriterIT.java bd00f02 
>   test/src/test/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java b78a311 
>   test/src/test/java/org/apache/accumulo/test/NoMutationRecoveryIT.java 6a9975c 
>   test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 56a6a70 
>   test/src/test/java/org/apache/accumulo/test/functional/WALSunnyDayIT.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/functional/WatchTheWatchCountIT.java bd0555b 
>   test/src/test/java/org/apache/accumulo/test/performance/RollWALPerformanceIT.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/replication/GarbageCollectorCommunicatesWithTServersIT.java 5b89d9c 
>   test/src/test/java/org/apache/accumulo/test/replication/MultiInstanceReplicationIT.java 9dec16e 
>   test/src/test/java/org/apache/accumulo/test/replication/ReplicationIT.java 54348db 
> 
> Diff: https://reviews.apache.org/r/32224/diff/
> 
> 
> Testing
> -------
> 
> Ran all tests, except RandomWalk.
> 
> 
> Thanks,
> 
> Eric Newton
> 
>


Re: Review Request 32224: ACCUMULO-3423

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


This change make the case where a tablet has some non-empty WALs with no data more likely (not sure how likely it would have been before).  For example a tablet could have WAL1 with data for it, WAL2 with no data for it, and WAL3 with data.   Seems like continuous ingest would not trigger this case.  Wonder if an IT for this case is worthwhile.

I am still reviewing.  Was getting nervous about RB losing my comments, so posting some comments I made so far.


core/src/main/java/org/apache/accumulo/core/metadata/schema/MetadataSchema.java
<https://reviews.apache.org/r/32224/#comment125109>

    Could have a stronger sanity check, check that `row.startsWith(section.getRowPrefix())`



core/src/test/java/org/apache/accumulo/core/metadata/MetadataTableSchemaTest.java
<https://reviews.apache.org/r/32224/#comment125111>

    Could add test for malformed entries



server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java
<https://reviews.apache.org/r/32224/#comment125125>

    IT that test volume replacement and log recovery, would be very nice if it does not exists.  This would test if volume replacement works w/ these new ~wal+ metadata table entries.



server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataStateStore.java
<https://reviews.apache.org/r/32224/#comment125126>

    Seems like we could end with logs for multiple tservers in the following case.
    
      1. Tablet T1 is assigned to Tserver1
      2. Tserver1.I1 dies
      3. Master adds logs for Tserver1 to Tablet T1
      4. Master assigns Tablet T1 to Tserver2
      5. Before Tserver T2 loads and recovers Tablet T1, it dies
      6. Master adds logs for Tserver2 to Tablet T1
    
    In this case Tablet T1 has walog from two tservers.  Not sure how well recovery code would handle this.   Its seems like recovery code makes the assumption that walogs are from one tserver.



server/base/src/main/java/org/apache/accumulo/server/util/ListVolumesUsed.java
<https://reviews.apache.org/r/32224/#comment125110>

    Need to add code to ListVolumesUsed to scan ~wal+ section of metdata table for volumes.



server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java
<https://reviews.apache.org/r/32224/#comment125127>

    If no tablets are assigned to a dead tserver, will its logs be marked as unused?  Was wondering if this operation is idempotent.  If the master dies right after flushChanges, then it seems like no tablets would be assigned to the dead server.


- kturner


On March 19, 2015, 4:42 p.m., Eric Newton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32224/
> -----------------------------------------------------------
> 
> (Updated March 19, 2015, 4:42 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-3423
>     https://issues.apache.org/jira/browse/ACCUMULO-3423
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Faster WAL rollovers
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/client/impl/ReplicationOperationsImpl.java 6a5c74a 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java 2403915 
>   core/src/main/java/org/apache/accumulo/core/metadata/RootTable.java 24148b1 
>   core/src/main/java/org/apache/accumulo/core/metadata/schema/MetadataSchema.java 534dd7f 
>   core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java 25d0f32 
>   core/src/test/java/org/apache/accumulo/core/metadata/MetadataTableSchemaTest.java PRE-CREATION 
>   server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java fc26ca4 
>   server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java e5bdded 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataStateStore.java 7ee6f0c 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataTableScanner.java 9be5a67 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/TabletLocationState.java b24b562 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/TabletStateStore.java 5413e31 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/ZooTabletStateStore.java ab99396 
>   server/base/src/main/java/org/apache/accumulo/server/replication/StatusUtil.java 898e3d4 
>   server/base/src/main/java/org/apache/accumulo/server/util/ListVolumesUsed.java e90d1dd 
>   server/base/src/main/java/org/apache/accumulo/server/util/MasterMetadataUtil.java 4ca2d64 
>   server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java 10cd749 
>   server/base/src/main/java/org/apache/accumulo/server/util/ReplicationTableUtil.java af02a8d 
>   server/base/src/test/java/org/apache/accumulo/server/util/ReplicationTableUtilTest.java b1010c2 
>   server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java 1735c0d 
>   server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java c8d5cd6 
>   server/gc/src/main/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferences.java 3a32727 
>   server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogsTest.java 5801faa 
>   server/gc/src/test/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferencesTest.java 23db83a 
>   server/master/src/main/java/org/apache/accumulo/master/Master.java 3762f32 
>   server/master/src/main/java/org/apache/accumulo/master/MasterClientServiceHandler.java f73c236 
>   server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java d097d75 
>   server/master/src/main/java/org/apache/accumulo/master/replication/WorkMaker.java 8532e1b 
>   server/master/src/main/java/org/apache/accumulo/master/state/MergeStats.java 44f229e 
>   server/master/src/test/java/org/apache/accumulo/master/ReplicationOperationsImplTest.java a127dcd 
>   server/master/src/test/java/org/apache/accumulo/master/TestMergeState.java b0240f1 
>   server/master/src/test/java/org/apache/accumulo/master/state/RootTabletStateStoreTest.java abceae4 
>   server/tserver/src/main/findbugs/exclude-filter.xml 47dd1f5 
>   server/tserver/src/main/java/org/apache/accumulo/server/GarbageCollectionLogger.java 8f3785e 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletLevel.java PRE-CREATION 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java 662ee31 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java 5acf5eb 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/SortedLogRecovery.java c4d9fab 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java 711c497 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CommitSession.java b4814e4 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/DatafileManager.java 594d9c5 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java 6152500 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/TabletCommitter.java 4bc05a6 
>   test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java b429607 
>   test/src/test/java/org/apache/accumulo/proxy/ProxyDurabilityIT.java 404a8fd 
>   test/src/test/java/org/apache/accumulo/test/BadDeleteMarkersCreatedIT.java 25337b2 
>   test/src/test/java/org/apache/accumulo/test/BalanceIT.java f793925 
>   test/src/test/java/org/apache/accumulo/test/CleanWalIT.java f553be8 
>   test/src/test/java/org/apache/accumulo/test/ConditionalWriterIT.java bd00f02 
>   test/src/test/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java b78a311 
>   test/src/test/java/org/apache/accumulo/test/NoMutationRecoveryIT.java 6a9975c 
>   test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 56a6a70 
>   test/src/test/java/org/apache/accumulo/test/functional/WALSunnyDayIT.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/functional/WatchTheWatchCountIT.java bd0555b 
>   test/src/test/java/org/apache/accumulo/test/performance/RollWALPerformanceIT.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/replication/GarbageCollectorCommunicatesWithTServersIT.java 5b89d9c 
>   test/src/test/java/org/apache/accumulo/test/replication/MultiInstanceReplicationIT.java 9dec16e 
>   test/src/test/java/org/apache/accumulo/test/replication/ReplicationIT.java 54348db 
> 
> Diff: https://reviews.apache.org/r/32224/diff/
> 
> 
> Testing
> -------
> 
> Ran all tests, except RandomWalk.
> 
> 
> Thanks,
> 
> Eric Newton
> 
>


Re: Review Request 32224: ACCUMULO-3423

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



server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
<https://reviews.apache.org/r/32224/#comment126538>

    Could name this `markUnusedWALs()` instead of `removeUnusedWALs()`?



test/src/test/java/org/apache/accumulo/test/functional/WALSunnyDayIT.java
<https://reviews.apache.org/r/32224/#comment126541>

    Would be good to read and verify data before writing anything.


- kturner


On March 19, 2015, 4:42 p.m., Eric Newton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32224/
> -----------------------------------------------------------
> 
> (Updated March 19, 2015, 4:42 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-3423
>     https://issues.apache.org/jira/browse/ACCUMULO-3423
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Faster WAL rollovers
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/client/impl/ReplicationOperationsImpl.java 6a5c74a 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java 2403915 
>   core/src/main/java/org/apache/accumulo/core/metadata/RootTable.java 24148b1 
>   core/src/main/java/org/apache/accumulo/core/metadata/schema/MetadataSchema.java 534dd7f 
>   core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java 25d0f32 
>   core/src/test/java/org/apache/accumulo/core/metadata/MetadataTableSchemaTest.java PRE-CREATION 
>   server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java fc26ca4 
>   server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java e5bdded 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataStateStore.java 7ee6f0c 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataTableScanner.java 9be5a67 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/TabletLocationState.java b24b562 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/TabletStateStore.java 5413e31 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/ZooTabletStateStore.java ab99396 
>   server/base/src/main/java/org/apache/accumulo/server/replication/StatusUtil.java 898e3d4 
>   server/base/src/main/java/org/apache/accumulo/server/util/ListVolumesUsed.java e90d1dd 
>   server/base/src/main/java/org/apache/accumulo/server/util/MasterMetadataUtil.java 4ca2d64 
>   server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java 10cd749 
>   server/base/src/main/java/org/apache/accumulo/server/util/ReplicationTableUtil.java af02a8d 
>   server/base/src/test/java/org/apache/accumulo/server/util/ReplicationTableUtilTest.java b1010c2 
>   server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java 1735c0d 
>   server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java c8d5cd6 
>   server/gc/src/main/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferences.java 3a32727 
>   server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogsTest.java 5801faa 
>   server/gc/src/test/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferencesTest.java 23db83a 
>   server/master/src/main/java/org/apache/accumulo/master/Master.java 3762f32 
>   server/master/src/main/java/org/apache/accumulo/master/MasterClientServiceHandler.java f73c236 
>   server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java d097d75 
>   server/master/src/main/java/org/apache/accumulo/master/replication/WorkMaker.java 8532e1b 
>   server/master/src/main/java/org/apache/accumulo/master/state/MergeStats.java 44f229e 
>   server/master/src/test/java/org/apache/accumulo/master/ReplicationOperationsImplTest.java a127dcd 
>   server/master/src/test/java/org/apache/accumulo/master/TestMergeState.java b0240f1 
>   server/master/src/test/java/org/apache/accumulo/master/state/RootTabletStateStoreTest.java abceae4 
>   server/tserver/src/main/findbugs/exclude-filter.xml 47dd1f5 
>   server/tserver/src/main/java/org/apache/accumulo/server/GarbageCollectionLogger.java 8f3785e 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletLevel.java PRE-CREATION 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java 662ee31 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java 5acf5eb 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/SortedLogRecovery.java c4d9fab 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java 711c497 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CommitSession.java b4814e4 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/DatafileManager.java 594d9c5 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java 6152500 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/TabletCommitter.java 4bc05a6 
>   test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java b429607 
>   test/src/test/java/org/apache/accumulo/proxy/ProxyDurabilityIT.java 404a8fd 
>   test/src/test/java/org/apache/accumulo/test/BadDeleteMarkersCreatedIT.java 25337b2 
>   test/src/test/java/org/apache/accumulo/test/BalanceIT.java f793925 
>   test/src/test/java/org/apache/accumulo/test/CleanWalIT.java f553be8 
>   test/src/test/java/org/apache/accumulo/test/ConditionalWriterIT.java bd00f02 
>   test/src/test/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java b78a311 
>   test/src/test/java/org/apache/accumulo/test/NoMutationRecoveryIT.java 6a9975c 
>   test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 56a6a70 
>   test/src/test/java/org/apache/accumulo/test/functional/WALSunnyDayIT.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/functional/WatchTheWatchCountIT.java bd0555b 
>   test/src/test/java/org/apache/accumulo/test/performance/RollWALPerformanceIT.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/replication/GarbageCollectorCommunicatesWithTServersIT.java 5b89d9c 
>   test/src/test/java/org/apache/accumulo/test/replication/MultiInstanceReplicationIT.java 9dec16e 
>   test/src/test/java/org/apache/accumulo/test/replication/ReplicationIT.java 54348db 
> 
> Diff: https://reviews.apache.org/r/32224/diff/
> 
> 
> Testing
> -------
> 
> Ran all tests, except RandomWalk.
> 
> 
> Thanks,
> 
> Eric Newton
> 
>


Re: Review Request 32224: ACCUMULO-3423

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


I am still reviewing.  I think I have covered Master, GC code, and a little bit of tserver code.   Still want to take a more indepth look at tserver code and test.


server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java
<https://reviews.apache.org/r/32224/#comment125714>

    I think catching this exception and continuing will cause the candidate to be removed from metatadata table.  Is this what you wanted?  Could possibly leave it in metadata table, so it would be a candidate again on the next GC run.



server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java
<https://reviews.apache.org/r/32224/#comment125712>

    I think `removeReferencedCandidates()` or `removeInUseCandidates()` would be a better name for this.  If this method name is changed, could rename `removeMarkers()` to `removeMetadataEntries()` :)



server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java
<https://reviews.apache.org/r/32224/#comment125707>

    Feel like using UUIDs would be safer



server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java
<https://reviews.apache.org/r/32224/#comment125715>

    TODO for me.  I have not reviewed replication stuff yet.  Wanted to discuss w/ you and get a quick conceptual overview first.



server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java
<https://reviews.apache.org/r/32224/#comment125711>

    This code should be extemely mistrustful of the data its reading.  `CurrentLogsSection.getTabletServer()` does some sanity checks of the data.  What other sanity checks could be done here?  Seems like some validation that the paths is as expected could be done.  Also if strong validation of the tserver instance data is not, then it would be nice to validate that.



server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java
<https://reviews.apache.org/r/32224/#comment125700>

    whats the purpose of `!rootWALs.contains(path)`



server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java
<https://reviews.apache.org/r/32224/#comment125706>

    Not sure, I think the old code only dealt with UUIDs for determining what was in use.  It seems like the chanes are comparing paths and not uuids?  
    
    Comparing UUIDs was important for relatavie paths and also paths that are normalized differently.   Maybe using hadoop will normalize things the same.. and maybe relative paths are no longer a concern? If so should check.   
    
    Thinkin using only UUIDs for determiing in use status is still safest, sometimes two properly normalized paths can still be different but refer to the same thing.


- kturner


On March 19, 2015, 4:42 p.m., Eric Newton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32224/
> -----------------------------------------------------------
> 
> (Updated March 19, 2015, 4:42 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-3423
>     https://issues.apache.org/jira/browse/ACCUMULO-3423
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Faster WAL rollovers
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/client/impl/ReplicationOperationsImpl.java 6a5c74a 
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java 2403915 
>   core/src/main/java/org/apache/accumulo/core/metadata/RootTable.java 24148b1 
>   core/src/main/java/org/apache/accumulo/core/metadata/schema/MetadataSchema.java 534dd7f 
>   core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java 25d0f32 
>   core/src/test/java/org/apache/accumulo/core/metadata/MetadataTableSchemaTest.java PRE-CREATION 
>   server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java fc26ca4 
>   server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java e5bdded 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataStateStore.java 7ee6f0c 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataTableScanner.java 9be5a67 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/TabletLocationState.java b24b562 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/TabletStateStore.java 5413e31 
>   server/base/src/main/java/org/apache/accumulo/server/master/state/ZooTabletStateStore.java ab99396 
>   server/base/src/main/java/org/apache/accumulo/server/replication/StatusUtil.java 898e3d4 
>   server/base/src/main/java/org/apache/accumulo/server/util/ListVolumesUsed.java e90d1dd 
>   server/base/src/main/java/org/apache/accumulo/server/util/MasterMetadataUtil.java 4ca2d64 
>   server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java 10cd749 
>   server/base/src/main/java/org/apache/accumulo/server/util/ReplicationTableUtil.java af02a8d 
>   server/base/src/test/java/org/apache/accumulo/server/util/ReplicationTableUtilTest.java b1010c2 
>   server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java 1735c0d 
>   server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java c8d5cd6 
>   server/gc/src/main/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferences.java 3a32727 
>   server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogsTest.java 5801faa 
>   server/gc/src/test/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferencesTest.java 23db83a 
>   server/master/src/main/java/org/apache/accumulo/master/Master.java 3762f32 
>   server/master/src/main/java/org/apache/accumulo/master/MasterClientServiceHandler.java f73c236 
>   server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java d097d75 
>   server/master/src/main/java/org/apache/accumulo/master/replication/WorkMaker.java 8532e1b 
>   server/master/src/main/java/org/apache/accumulo/master/state/MergeStats.java 44f229e 
>   server/master/src/test/java/org/apache/accumulo/master/ReplicationOperationsImplTest.java a127dcd 
>   server/master/src/test/java/org/apache/accumulo/master/TestMergeState.java b0240f1 
>   server/master/src/test/java/org/apache/accumulo/master/state/RootTabletStateStoreTest.java abceae4 
>   server/tserver/src/main/findbugs/exclude-filter.xml 47dd1f5 
>   server/tserver/src/main/java/org/apache/accumulo/server/GarbageCollectionLogger.java 8f3785e 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletLevel.java PRE-CREATION 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java 662ee31 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java 5acf5eb 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/SortedLogRecovery.java c4d9fab 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java 711c497 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CommitSession.java b4814e4 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/DatafileManager.java 594d9c5 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java 6152500 
>   server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/TabletCommitter.java 4bc05a6 
>   test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java b429607 
>   test/src/test/java/org/apache/accumulo/proxy/ProxyDurabilityIT.java 404a8fd 
>   test/src/test/java/org/apache/accumulo/test/BadDeleteMarkersCreatedIT.java 25337b2 
>   test/src/test/java/org/apache/accumulo/test/BalanceIT.java f793925 
>   test/src/test/java/org/apache/accumulo/test/CleanWalIT.java f553be8 
>   test/src/test/java/org/apache/accumulo/test/ConditionalWriterIT.java bd00f02 
>   test/src/test/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java b78a311 
>   test/src/test/java/org/apache/accumulo/test/NoMutationRecoveryIT.java 6a9975c 
>   test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 56a6a70 
>   test/src/test/java/org/apache/accumulo/test/functional/WALSunnyDayIT.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/functional/WatchTheWatchCountIT.java bd0555b 
>   test/src/test/java/org/apache/accumulo/test/performance/RollWALPerformanceIT.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/replication/GarbageCollectorCommunicatesWithTServersIT.java 5b89d9c 
>   test/src/test/java/org/apache/accumulo/test/replication/MultiInstanceReplicationIT.java 9dec16e 
>   test/src/test/java/org/apache/accumulo/test/replication/ReplicationIT.java 54348db 
> 
> Diff: https://reviews.apache.org/r/32224/diff/
> 
> 
> Testing
> -------
> 
> Ran all tests, except RandomWalk.
> 
> 
> Thanks,
> 
> Eric Newton
> 
>


Re: Review Request 32224: ACCUMULO-3423

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

(Updated March 19, 2015, 12:42 p.m.)


Review request for accumulo.


Bugs: ACCUMULO-3423
    https://issues.apache.org/jira/browse/ACCUMULO-3423


Repository: accumulo


Description
-------

Faster WAL rollovers


Diffs
-----

  core/src/main/java/org/apache/accumulo/core/client/impl/ReplicationOperationsImpl.java 6a5c74a 
  core/src/main/java/org/apache/accumulo/core/conf/Property.java 2403915 
  core/src/main/java/org/apache/accumulo/core/metadata/RootTable.java 24148b1 
  core/src/main/java/org/apache/accumulo/core/metadata/schema/MetadataSchema.java 534dd7f 
  core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java 25d0f32 
  core/src/test/java/org/apache/accumulo/core/metadata/MetadataTableSchemaTest.java PRE-CREATION 
  server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java fc26ca4 
  server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java e5bdded 
  server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataStateStore.java 7ee6f0c 
  server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataTableScanner.java 9be5a67 
  server/base/src/main/java/org/apache/accumulo/server/master/state/TabletLocationState.java b24b562 
  server/base/src/main/java/org/apache/accumulo/server/master/state/TabletStateStore.java 5413e31 
  server/base/src/main/java/org/apache/accumulo/server/master/state/ZooTabletStateStore.java ab99396 
  server/base/src/main/java/org/apache/accumulo/server/replication/StatusUtil.java 898e3d4 
  server/base/src/main/java/org/apache/accumulo/server/util/ListVolumesUsed.java e90d1dd 
  server/base/src/main/java/org/apache/accumulo/server/util/MasterMetadataUtil.java 4ca2d64 
  server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java 10cd749 
  server/base/src/main/java/org/apache/accumulo/server/util/ReplicationTableUtil.java af02a8d 
  server/base/src/test/java/org/apache/accumulo/server/util/ReplicationTableUtilTest.java b1010c2 
  server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java 1735c0d 
  server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java c8d5cd6 
  server/gc/src/main/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferences.java 3a32727 
  server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogsTest.java 5801faa 
  server/gc/src/test/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferencesTest.java 23db83a 
  server/master/src/main/java/org/apache/accumulo/master/Master.java 3762f32 
  server/master/src/main/java/org/apache/accumulo/master/MasterClientServiceHandler.java f73c236 
  server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java d097d75 
  server/master/src/main/java/org/apache/accumulo/master/replication/WorkMaker.java 8532e1b 
  server/master/src/main/java/org/apache/accumulo/master/state/MergeStats.java 44f229e 
  server/master/src/test/java/org/apache/accumulo/master/ReplicationOperationsImplTest.java a127dcd 
  server/master/src/test/java/org/apache/accumulo/master/TestMergeState.java b0240f1 
  server/master/src/test/java/org/apache/accumulo/master/state/RootTabletStateStoreTest.java abceae4 
  server/tserver/src/main/findbugs/exclude-filter.xml 47dd1f5 
  server/tserver/src/main/java/org/apache/accumulo/server/GarbageCollectionLogger.java 8f3785e 
  server/tserver/src/main/java/org/apache/accumulo/tserver/TabletLevel.java PRE-CREATION 
  server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java 662ee31 
  server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java 5acf5eb 
  server/tserver/src/main/java/org/apache/accumulo/tserver/log/SortedLogRecovery.java c4d9fab 
  server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java 711c497 
  server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CommitSession.java b4814e4 
  server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/DatafileManager.java 594d9c5 
  server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java 6152500 
  server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/TabletCommitter.java 4bc05a6 
  test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java b429607 
  test/src/test/java/org/apache/accumulo/proxy/ProxyDurabilityIT.java 404a8fd 
  test/src/test/java/org/apache/accumulo/test/BadDeleteMarkersCreatedIT.java 25337b2 
  test/src/test/java/org/apache/accumulo/test/BalanceIT.java f793925 
  test/src/test/java/org/apache/accumulo/test/CleanWalIT.java f553be8 
  test/src/test/java/org/apache/accumulo/test/ConditionalWriterIT.java bd00f02 
  test/src/test/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java b78a311 
  test/src/test/java/org/apache/accumulo/test/NoMutationRecoveryIT.java 6a9975c 
  test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 56a6a70 
  test/src/test/java/org/apache/accumulo/test/functional/WALSunnyDayIT.java PRE-CREATION 
  test/src/test/java/org/apache/accumulo/test/functional/WatchTheWatchCountIT.java bd0555b 
  test/src/test/java/org/apache/accumulo/test/performance/RollWALPerformanceIT.java PRE-CREATION 
  test/src/test/java/org/apache/accumulo/test/replication/GarbageCollectorCommunicatesWithTServersIT.java 5b89d9c 
  test/src/test/java/org/apache/accumulo/test/replication/MultiInstanceReplicationIT.java 9dec16e 
  test/src/test/java/org/apache/accumulo/test/replication/ReplicationIT.java 54348db 

Diff: https://reviews.apache.org/r/32224/diff/


Testing
-------

Ran all tests, except RandomWalk.


Thanks,

Eric Newton


Re: Review Request 32224: ACCUMULO-3423

Posted by Josh Elser <jo...@gmail.com>.
Thanks for the heads up. FYI, I plan to give it another pass tonight or 
tmrw.

Eric Newton wrote:
> I know this thing is huge.  Josh has already been over it a couple of times.
>
> I plan to push it to master very soon.  If you would like to hold off for a
> more intensive review, please let me know.
>
> At this point, I've run the tests so many times, the ITs are more stable in
> my branch than in master.
>
> You can also find this at github:
>
> https://github.com/ericnewton/accumulo-3423
>
> -Eric
>
>
> On Wed, Mar 18, 2015 at 7:58 PM, Eric Newton<er...@gmail.com>  wrote:
>
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/32224/
>> -----------------------------------------------------------
>>
>> Review request for accumulo.
>>
>>
>> Repository: accumulo
>>
>>
>> Description
>> -------
>>
>> Faster WAL rollovers
>>
>>
>> Diffs
>> -----
>>
>>
>> core/src/main/java/org/apache/accumulo/core/client/impl/ReplicationOperationsImpl.java
>> 6a5c74a
>>    core/src/main/java/org/apache/accumulo/core/conf/Property.java 2403915
>>    core/src/main/java/org/apache/accumulo/core/metadata/RootTable.java
>> 24148b1
>>
>> core/src/main/java/org/apache/accumulo/core/metadata/schema/MetadataSchema.java
>> 534dd7f
>>
>> core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java
>> 25d0f32
>>
>> core/src/test/java/org/apache/accumulo/core/metadata/MetadataTableSchemaTest.java
>> PRE-CREATION
>>    server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java
>> fc26ca4
>>
>> server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java
>> e5bdded
>>
>> server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataStateStore.java
>> 7ee6f0c
>>
>> server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataTableScanner.java
>> 9be5a67
>>
>> server/base/src/main/java/org/apache/accumulo/server/master/state/TabletLocationState.java
>> b24b562
>>
>> server/base/src/main/java/org/apache/accumulo/server/master/state/TabletStateStore.java
>> 5413e31
>>
>> server/base/src/main/java/org/apache/accumulo/server/master/state/ZooTabletStateStore.java
>> ab99396
>>
>> server/base/src/main/java/org/apache/accumulo/server/replication/StatusUtil.java
>> 898e3d4
>>
>> server/base/src/main/java/org/apache/accumulo/server/util/ListVolumesUsed.java
>> e90d1dd
>>
>> server/base/src/main/java/org/apache/accumulo/server/util/MasterMetadataUtil.java
>> 4ca2d64
>>
>> server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java
>> 10cd749
>>
>> server/base/src/main/java/org/apache/accumulo/server/util/ReplicationTableUtil.java
>> af02a8d
>>
>> server/base/src/test/java/org/apache/accumulo/server/util/ReplicationTableUtilTest.java
>> b1010c2
>>
>> server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java
>> 1735c0d
>>
>> server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java
>> c8d5cd6
>>
>> server/gc/src/main/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferences.java
>> 3a32727
>>
>> server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogsTest.java
>> 5801faa
>>
>> server/gc/src/test/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferencesTest.java
>> 23db83a
>>    server/master/src/main/java/org/apache/accumulo/master/Master.java
>> 3762f32
>>
>> server/master/src/main/java/org/apache/accumulo/master/MasterClientServiceHandler.java
>> f73c236
>>
>> server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java
>> d097d75
>>
>> server/master/src/main/java/org/apache/accumulo/master/replication/WorkMaker.java
>> 8532e1b
>>
>> server/master/src/main/java/org/apache/accumulo/master/state/MergeStats.java
>> 44f229e
>>
>> server/master/src/test/java/org/apache/accumulo/master/ReplicationOperationsImplTest.java
>> a127dcd
>>
>> server/master/src/test/java/org/apache/accumulo/master/TestMergeState.java
>> b0240f1
>>
>> server/master/src/test/java/org/apache/accumulo/master/state/RootTabletStateStoreTest.java
>> abceae4
>>    server/tserver/src/main/findbugs/exclude-filter.xml 47dd1f5
>>
>> server/tserver/src/main/java/org/apache/accumulo/server/GarbageCollectionLogger.java
>> 8f3785e
>>
>> server/tserver/src/main/java/org/apache/accumulo/tserver/TabletLevel.java
>> PRE-CREATION
>>
>> server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
>> 662ee31
>>
>> server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java
>> 5acf5eb
>>
>> server/tserver/src/main/java/org/apache/accumulo/tserver/log/SortedLogRecovery.java
>> c4d9fab
>>
>> server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java
>> 711c497
>>
>> server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CommitSession.java
>> b4814e4
>>
>> server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/DatafileManager.java
>> 594d9c5
>>
>> server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java
>> 6152500
>>
>> server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/TabletCommitter.java
>> 4bc05a6
>>
>> test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java
>> b429607
>>    test/src/test/java/org/apache/accumulo/proxy/ProxyDurabilityIT.java
>> 404a8fd
>>
>> test/src/test/java/org/apache/accumulo/test/BadDeleteMarkersCreatedIT.java
>> 25337b2
>>    test/src/test/java/org/apache/accumulo/test/BalanceIT.java f793925
>>    test/src/test/java/org/apache/accumulo/test/CleanWalIT.java f553be8
>>    test/src/test/java/org/apache/accumulo/test/ConditionalWriterIT.java
>> bd00f02
>>
>> test/src/test/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java
>> b78a311
>>    test/src/test/java/org/apache/accumulo/test/NoMutationRecoveryIT.java
>> 6a9975c
>>    test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 56a6a70
>>
>> test/src/test/java/org/apache/accumulo/test/functional/WALSunnyDayIT.java
>> PRE-CREATION
>>
>> test/src/test/java/org/apache/accumulo/test/functional/WatchTheWatchCountIT.java
>> bd0555b
>>
>> test/src/test/java/org/apache/accumulo/test/performance/RollWALPerformanceIT.java
>> PRE-CREATION
>>
>> test/src/test/java/org/apache/accumulo/test/replication/GarbageCollectorCommunicatesWithTServersIT.java
>> 5b89d9c
>>
>> test/src/test/java/org/apache/accumulo/test/replication/MultiInstanceReplicationIT.java
>> 9dec16e
>>
>> test/src/test/java/org/apache/accumulo/test/replication/ReplicationIT.java
>> 54348db
>>
>> Diff: https://reviews.apache.org/r/32224/diff/
>>
>>
>> Testing
>> -------
>>
>> Ran all tests, except RandomWalk.
>>
>>
>> Thanks,
>>
>> Eric Newton
>>
>>
>

Re: Review Request 32224: ACCUMULO-3423

Posted by Eric Newton <er...@gmail.com>.
I know this thing is huge.  Josh has already been over it a couple of times.

I plan to push it to master very soon.  If you would like to hold off for a
more intensive review, please let me know.

At this point, I've run the tests so many times, the ITs are more stable in
my branch than in master.

You can also find this at github:

https://github.com/ericnewton/accumulo-3423

-Eric


On Wed, Mar 18, 2015 at 7:58 PM, Eric Newton <er...@gmail.com> wrote:

>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32224/
> -----------------------------------------------------------
>
> Review request for accumulo.
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> Faster WAL rollovers
>
>
> Diffs
> -----
>
>
> core/src/main/java/org/apache/accumulo/core/client/impl/ReplicationOperationsImpl.java
> 6a5c74a
>   core/src/main/java/org/apache/accumulo/core/conf/Property.java 2403915
>   core/src/main/java/org/apache/accumulo/core/metadata/RootTable.java
> 24148b1
>
> core/src/main/java/org/apache/accumulo/core/metadata/schema/MetadataSchema.java
> 534dd7f
>
> core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java
> 25d0f32
>
> core/src/test/java/org/apache/accumulo/core/metadata/MetadataTableSchemaTest.java
> PRE-CREATION
>   server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java
> fc26ca4
>
> server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java
> e5bdded
>
> server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataStateStore.java
> 7ee6f0c
>
> server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataTableScanner.java
> 9be5a67
>
> server/base/src/main/java/org/apache/accumulo/server/master/state/TabletLocationState.java
> b24b562
>
> server/base/src/main/java/org/apache/accumulo/server/master/state/TabletStateStore.java
> 5413e31
>
> server/base/src/main/java/org/apache/accumulo/server/master/state/ZooTabletStateStore.java
> ab99396
>
> server/base/src/main/java/org/apache/accumulo/server/replication/StatusUtil.java
> 898e3d4
>
> server/base/src/main/java/org/apache/accumulo/server/util/ListVolumesUsed.java
> e90d1dd
>
> server/base/src/main/java/org/apache/accumulo/server/util/MasterMetadataUtil.java
> 4ca2d64
>
> server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java
> 10cd749
>
> server/base/src/main/java/org/apache/accumulo/server/util/ReplicationTableUtil.java
> af02a8d
>
> server/base/src/test/java/org/apache/accumulo/server/util/ReplicationTableUtilTest.java
> b1010c2
>
> server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogs.java
> 1735c0d
>
> server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java
> c8d5cd6
>
> server/gc/src/main/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferences.java
> 3a32727
>
> server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectWriteAheadLogsTest.java
> 5801faa
>
> server/gc/src/test/java/org/apache/accumulo/gc/replication/CloseWriteAheadLogReferencesTest.java
> 23db83a
>   server/master/src/main/java/org/apache/accumulo/master/Master.java
> 3762f32
>
> server/master/src/main/java/org/apache/accumulo/master/MasterClientServiceHandler.java
> f73c236
>
> server/master/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java
> d097d75
>
> server/master/src/main/java/org/apache/accumulo/master/replication/WorkMaker.java
> 8532e1b
>
> server/master/src/main/java/org/apache/accumulo/master/state/MergeStats.java
> 44f229e
>
> server/master/src/test/java/org/apache/accumulo/master/ReplicationOperationsImplTest.java
> a127dcd
>
> server/master/src/test/java/org/apache/accumulo/master/TestMergeState.java
> b0240f1
>
> server/master/src/test/java/org/apache/accumulo/master/state/RootTabletStateStoreTest.java
> abceae4
>   server/tserver/src/main/findbugs/exclude-filter.xml 47dd1f5
>
> server/tserver/src/main/java/org/apache/accumulo/server/GarbageCollectionLogger.java
> 8f3785e
>
> server/tserver/src/main/java/org/apache/accumulo/tserver/TabletLevel.java
> PRE-CREATION
>
> server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
> 662ee31
>
> server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java
> 5acf5eb
>
> server/tserver/src/main/java/org/apache/accumulo/tserver/log/SortedLogRecovery.java
> c4d9fab
>
> server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java
> 711c497
>
> server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CommitSession.java
> b4814e4
>
> server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/DatafileManager.java
> 594d9c5
>
> server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java
> 6152500
>
> server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/TabletCommitter.java
> 4bc05a6
>
> test/src/main/java/org/apache/accumulo/test/performance/thrift/NullTserver.java
> b429607
>   test/src/test/java/org/apache/accumulo/proxy/ProxyDurabilityIT.java
> 404a8fd
>
> test/src/test/java/org/apache/accumulo/test/BadDeleteMarkersCreatedIT.java
> 25337b2
>   test/src/test/java/org/apache/accumulo/test/BalanceIT.java f793925
>   test/src/test/java/org/apache/accumulo/test/CleanWalIT.java f553be8
>   test/src/test/java/org/apache/accumulo/test/ConditionalWriterIT.java
> bd00f02
>
> test/src/test/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java
> b78a311
>   test/src/test/java/org/apache/accumulo/test/NoMutationRecoveryIT.java
> 6a9975c
>   test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 56a6a70
>
> test/src/test/java/org/apache/accumulo/test/functional/WALSunnyDayIT.java
> PRE-CREATION
>
> test/src/test/java/org/apache/accumulo/test/functional/WatchTheWatchCountIT.java
> bd0555b
>
> test/src/test/java/org/apache/accumulo/test/performance/RollWALPerformanceIT.java
> PRE-CREATION
>
> test/src/test/java/org/apache/accumulo/test/replication/GarbageCollectorCommunicatesWithTServersIT.java
> 5b89d9c
>
> test/src/test/java/org/apache/accumulo/test/replication/MultiInstanceReplicationIT.java
> 9dec16e
>
> test/src/test/java/org/apache/accumulo/test/replication/ReplicationIT.java
> 54348db
>
> Diff: https://reviews.apache.org/r/32224/diff/
>
>
> Testing
> -------
>
> Ran all tests, except RandomWalk.
>
>
> Thanks,
>
> Eric Newton
>
>