accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ke...@deenlo.com
Subject Re: Review Request 32224: ACCUMULO-3423
Date Fri, 20 Mar 2015 19:11:21 GMT

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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message