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