-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-20461 Implement fsync for AsyncFSWAL #947
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -353,7 +353,7 @@ private void sync(AsyncWriter writer) { | |
| highestProcessedAppendTxidAtLastSync = currentHighestProcessedAppendTxid; | ||
| final long startTimeNs = System.nanoTime(); | ||
| final long epoch = (long) epochAndState >>> 2L; | ||
| addListener(writer.sync(), (result, error) -> { | ||
| addListener(writer.sync(useHsync), (result, error) -> { | ||
| if (error != null) { | ||
| syncFailed(epoch, error); | ||
| } else { | ||
|
|
@@ -630,11 +630,21 @@ protected long append(RegionInfo hri, WALKeyImpl key, WALEdit edits, boolean inM | |
|
|
||
| @Override | ||
| public void sync() throws IOException { | ||
| sync(useHsync); | ||
| } | ||
|
|
||
| @Override | ||
| public void sync(long txid) throws IOException { | ||
| sync(txid, useHsync); | ||
| } | ||
|
|
||
| @Override | ||
| public void sync(boolean forceSync) throws IOException { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this right? The method is sync. We pass a forceSync flag. There are conditions under which we won't sync when we call sync such that we might need a force?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, I see you are just using what was there... moving it up into abstract wal that this wal impl uses. Ok. The doc of the sync types can be afterward. |
||
| try (TraceScope scope = TraceUtil.createTrace("AsyncFSWAL.sync")) { | ||
| long txid = waitingConsumePayloads.next(); | ||
| SyncFuture future; | ||
| try { | ||
| future = getSyncFuture(txid); | ||
| future = getSyncFuture(txid, forceSync); | ||
| RingBufferTruck truck = waitingConsumePayloads.get(txid); | ||
| truck.load(future); | ||
| } finally { | ||
|
|
@@ -648,7 +658,7 @@ public void sync() throws IOException { | |
| } | ||
|
|
||
| @Override | ||
| public void sync(long txid) throws IOException { | ||
| public void sync(long txid, boolean forceSync) throws IOException { | ||
| if (highestSyncedTxid.get() >= txid) { | ||
| return; | ||
| } | ||
|
|
@@ -657,7 +667,7 @@ public void sync(long txid) throws IOException { | |
| long sequence = waitingConsumePayloads.next(); | ||
| SyncFuture future; | ||
| try { | ||
| future = getSyncFuture(txid); | ||
| future = getSyncFuture(txid, forceSync); | ||
| RingBufferTruck truck = waitingConsumePayloads.get(sequence); | ||
| truck.load(future); | ||
| } finally { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -85,7 +85,7 @@ interface Writer extends WriterBase { | |
| } | ||
|
|
||
| interface AsyncWriter extends WriterBase { | ||
| CompletableFuture<Long> sync(); | ||
| CompletableFuture<Long> sync(boolean forceSync); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I checked and WALProvider is audience private so this change is good. |
||
|
|
||
| void append(WAL.Entry entry); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,106 @@ | ||
| /** | ||
| * Licensed to the Apache Software Foundation (ASF) under one | ||
| * or more contributor license agreements. See the NOTICE file | ||
| * distributed with this work for additional information | ||
| * regarding copyright ownership. The ASF licenses this file | ||
| * to you under the Apache License, Version 2.0 (the | ||
| * "License"); you may not use this file except in compliance | ||
| * with the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package org.apache.hadoop.hbase.regionserver.wal; | ||
|
|
||
| import java.io.IOException; | ||
| import org.apache.hadoop.conf.Configuration; | ||
| import org.apache.hadoop.fs.FileSystem; | ||
| import org.apache.hadoop.fs.Path; | ||
| import org.apache.hadoop.hbase.HBaseClassTestRule; | ||
| import org.apache.hadoop.hbase.HConstants; | ||
| import org.apache.hadoop.hbase.regionserver.RegionServerServices; | ||
| import org.apache.hadoop.hbase.testclassification.MediumTests; | ||
| import org.junit.AfterClass; | ||
| import org.junit.BeforeClass; | ||
| import org.junit.ClassRule; | ||
| import org.junit.experimental.categories.Category; | ||
|
|
||
| import org.apache.hbase.thirdparty.io.netty.channel.Channel; | ||
| import org.apache.hbase.thirdparty.io.netty.channel.EventLoopGroup; | ||
| import org.apache.hbase.thirdparty.io.netty.channel.nio.NioEventLoopGroup; | ||
| import org.apache.hbase.thirdparty.io.netty.channel.socket.nio.NioSocketChannel; | ||
|
|
||
| @Category({ RegionServerServices.class, MediumTests.class }) | ||
| public class TestAsyncFSWALDurability extends WALDurabilityTestBase<CustomAsyncFSWAL> { | ||
|
|
||
| @ClassRule | ||
| public static final HBaseClassTestRule CLASS_RULE = | ||
| HBaseClassTestRule.forClass(TestAsyncFSWALDurability.class); | ||
|
|
||
| private static NioEventLoopGroup GROUP; | ||
|
|
||
| @BeforeClass | ||
| public static void setUpBeforeClass() { | ||
| GROUP = new NioEventLoopGroup(); | ||
| } | ||
|
|
||
| @AfterClass | ||
| public static void tearDownAfterClass() { | ||
| GROUP.shutdownGracefully(); | ||
| } | ||
|
|
||
| @Override | ||
| protected CustomAsyncFSWAL getWAL(FileSystem fs, Path root, String logDir, Configuration conf) | ||
| throws IOException { | ||
| CustomAsyncFSWAL wal = | ||
| new CustomAsyncFSWAL(fs, root, logDir, conf, GROUP, NioSocketChannel.class); | ||
| wal.init(); | ||
| return wal; | ||
| } | ||
|
|
||
| @Override | ||
| protected void resetSyncFlag(CustomAsyncFSWAL wal) { | ||
| wal.resetSyncFlag(); | ||
| } | ||
|
|
||
| @Override | ||
| protected Boolean getSyncFlag(CustomAsyncFSWAL wal) { | ||
| return wal.getSyncFlag(); | ||
| } | ||
| } | ||
|
|
||
| class CustomAsyncFSWAL extends AsyncFSWAL { | ||
| private Boolean syncFlag; | ||
|
|
||
| public CustomAsyncFSWAL(FileSystem fs, Path rootDir, String logDir, Configuration conf, | ||
| EventLoopGroup eventLoopGroup, Class<? extends Channel> channelClass) | ||
| throws FailedLogCloseException, IOException { | ||
| super(fs, rootDir, logDir, HConstants.HREGION_OLDLOGDIR_NAME, conf, null, true, null, null, | ||
| eventLoopGroup, channelClass); | ||
| } | ||
|
|
||
| @Override | ||
| public void sync(boolean forceSync) throws IOException { | ||
| syncFlag = forceSync; | ||
| super.sync(forceSync); | ||
| } | ||
|
|
||
| @Override | ||
| public void sync(long txid, boolean forceSync) throws IOException { | ||
| syncFlag = forceSync; | ||
| super.sync(txid, forceSync); | ||
| } | ||
|
|
||
| void resetSyncFlag() { | ||
| this.syncFlag = null; | ||
| } | ||
|
|
||
| Boolean getSyncFlag() { | ||
| return syncFlag; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| /** | ||
| * Licensed to the Apache Software Foundation (ASF) under one | ||
| * or more contributor license agreements. See the NOTICE file | ||
| * distributed with this work for additional information | ||
| * regarding copyright ownership. The ASF licenses this file | ||
| * to you under the Apache License, Version 2.0 (the | ||
| * "License"); you may not use this file except in compliance | ||
| * with the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package org.apache.hadoop.hbase.regionserver.wal; | ||
|
|
||
| import java.io.IOException; | ||
| import org.apache.hadoop.conf.Configuration; | ||
| import org.apache.hadoop.fs.FileSystem; | ||
| import org.apache.hadoop.fs.Path; | ||
| import org.apache.hadoop.hbase.HBaseClassTestRule; | ||
| import org.apache.hadoop.hbase.regionserver.RegionServerServices; | ||
| import org.apache.hadoop.hbase.testclassification.MediumTests; | ||
| import org.junit.ClassRule; | ||
| import org.junit.experimental.categories.Category; | ||
|
|
||
| @Category({ RegionServerServices.class, MediumTests.class }) | ||
| public class TestFSHLogDurability extends WALDurabilityTestBase<CustomFSHLog> { | ||
|
|
||
| @ClassRule | ||
| public static final HBaseClassTestRule CLASS_RULE = | ||
| HBaseClassTestRule.forClass(TestFSHLogDurability.class); | ||
|
|
||
| @Override | ||
| protected CustomFSHLog getWAL(FileSystem fs, Path root, String logDir, Configuration conf) | ||
| throws IOException { | ||
| CustomFSHLog wal = new CustomFSHLog(fs, root, logDir, conf); | ||
| wal.init(); | ||
| return wal; | ||
| } | ||
|
|
||
| @Override | ||
| protected void resetSyncFlag(CustomFSHLog wal) { | ||
| wal.resetSyncFlag(); | ||
| } | ||
|
|
||
| @Override | ||
| protected Boolean getSyncFlag(CustomFSHLog wal) { | ||
| return wal.getSyncFlag(); | ||
| } | ||
| } | ||
|
|
||
| class CustomFSHLog extends FSHLog { | ||
| private Boolean syncFlag; | ||
|
|
||
| public CustomFSHLog(FileSystem fs, Path root, String logDir, Configuration conf) | ||
| throws IOException { | ||
| super(fs, root, logDir, conf); | ||
| } | ||
|
|
||
| @Override | ||
| public void sync(boolean forceSync) throws IOException { | ||
| syncFlag = forceSync; | ||
| super.sync(forceSync); | ||
| } | ||
|
|
||
| @Override | ||
| public void sync(long txid, boolean forceSync) throws IOException { | ||
| syncFlag = forceSync; | ||
| super.sync(txid, forceSync); | ||
| } | ||
|
|
||
| void resetSyncFlag() { | ||
| this.syncFlag = null; | ||
| } | ||
|
|
||
| Boolean getSyncFlag() { | ||
| return syncFlag; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I always get these confused? You can't just call it sync? Whats diff between hsync, sync, and fsync again? I forgot?