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

Reply via email to