Return-Path: X-Original-To: apmail-accumulo-dev-archive@www.apache.org Delivered-To: apmail-accumulo-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 0E37B17A54 for ; Fri, 20 Mar 2015 19:11:22 +0000 (UTC) Received: (qmail 22905 invoked by uid 500); 20 Mar 2015 19:11:21 -0000 Delivered-To: apmail-accumulo-dev-archive@accumulo.apache.org Received: (qmail 22865 invoked by uid 500); 20 Mar 2015 19:11:21 -0000 Mailing-List: contact dev-help@accumulo.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@accumulo.apache.org Delivered-To: mailing list dev@accumulo.apache.org Received: (qmail 22852 invoked by uid 99); 20 Mar 2015 19:11:21 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 20 Mar 2015 19:11:21 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id 670901D4627; Fri, 20 Mar 2015 19:11:21 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============1670512971980757082==" MIME-Version: 1.0 Subject: Re: Review Request 32224: ACCUMULO-3423 From: keith@deenlo.com To: "Eric Newton" , "accumulo" , keith@deenlo.com Date: Fri, 20 Mar 2015 19:11:21 -0000 Message-ID: <20150320191121.1489.57115@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org/ Auto-Submitted: auto-generated Sender: noreply@reviews.apache.org X-ReviewGroup: accumulo X-ReviewRequest-URL: https://reviews.apache.org/r/32224/ X-Sender: noreply@reviews.apache.org References: <20150319164257.19861.83455@reviews.apache.org> In-Reply-To: <20150319164257.19861.83455@reviews.apache.org> Reply-To: keith@deenlo.com X-ReviewRequest-Repository: accumulo --===============1670512971980757082== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit ----------------------------------------------------------- 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 Could have a stronger sanity check, check that `row.startsWith(section.getRowPrefix())` core/src/test/java/org/apache/accumulo/core/metadata/MetadataTableSchemaTest.java Could add test for malformed entries server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java 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 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 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 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 > > --===============1670512971980757082==--