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