HDDS-11239. Fix KeyOutputStream's exception handling when calling hsync concurrently #7047

duongkame · 2024-08-08T00:59:56Z

What changes were proposed in this pull request?

When a block (or BlockOutputStream, I'll use them interchangeably in this description) becomes faulty (any exception happens when sending requests to the pipeline), KeyOutputStream will allocate a new Block (as the current block), and hand off all pending changes in the BufferPool to the current block. This is done by KeyOutputStream's handleException.

Two essential changes are needed when allowing KeyOutputStream to be used by multiple threads concurrently.

When hsync is called concurrently, handleException can be called multiple times for a given block. This is different from when KeyOutputStream is fully synchronized. The second handleException on a block call will see the block has been closed already and some assertion that is true in full synchronization is no longer stand.

2. When the first thread finishes calling handleException, it can continue sending normal writes/hsync to the KeyOutputStream while exception handling may be still going on for other threads. These calls will mess up with the ongoing exception handling.

We need to isolate the current block if it's handling retry handoff. When a block is created to handle retry, it must block all the normal calls (write and hsync) until the last (faulty) block finishes the handoff.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-11239

How was this patch tested?

New tests were added to verify the failure and the fix: TestHSync#testConcurrentExceptionHandling

Before fix: https://github.com/duongkame/ozone/actions/runs/10308913291
After fix:
run 1: 20 x 1 https://github.com/duongkame/ozone/actions/runs/10427687646
run 2: 10x 10 https://github.com/duongkame/ozone/actions/runs/10428271747

Regression test on normal concurrent hsync: https://github.com/duongkame/ozone/actions/runs/10428273927

duongkame · 2024-08-16T20:05:04Z

I'll rebase when finishing the test on this branch.

hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/KeyOutputStream.java

jojochuang · 2024-08-16T20:50:23Z

...ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/BlockOutputStreamEntryPool.java

@@ -379,7 +380,7 @@ BlockOutputStreamEntry getCurrentStreamEntry() {
   * @return the new current open stream to write to


update javadoc for the new parameter. When should it be true/false?

hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/KeyOutputStream.java

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java

hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/BlockOutputStreamEntry.java

hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/KeyOutputStream.java

hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockOutputStream.java

jojochuang

apart from a few questions it looks good.

hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockOutputStream.java

...a/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java

jojochuang · 2024-08-20T18:00:32Z

M M IS: Inconsistent synchronization of org.apache.hadoop.hdds.scm.storage.BlockOutputStream.xceiverClientFactory; locked 60% of time Unsynchronized access at BlockOutputStream.java:[line 575]

duongkame · 2024-08-20T22:32:00Z

Did a rebase and reran the repeating test.
https://github.com/duongkame/ozone/actions/runs/10475934264
https://github.com/duongkame/ozone/actions/runs/10475943780

jojochuang

+1

...a/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java

jojochuang · 2024-08-21T06:58:39Z

seems to be failing consistently: TestHSync.testUncommittedBlocks:515 expected: but was:

I wonder if it's related to the new commit just merged #7074

chungen0126 · 2024-08-21T08:44:42Z

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSync.java

+        writes = runConcurrentWriteHSyncWithException(file, out, data, syncerThreads, errors, errorInjector);
+      }
+      validateWrittenFile(file, fs, data, writes);
+      fs.delete(file, false);


seems to be failing consistently: TestHSync.testUncommittedBlocks:515 expected: but was:

I wonder if it's related to the new commit just merged #7074

To address the failure in TestHsync#testUncommittedBlocks, ensure that the deletedTable is empty after fs.delete(file, false);
Rebase to master and add a line after fs.delete(file, false); to wait KeyDeletingService to clean up the deletedTable.

waitForEmptyDeletedTable();

…_FILE_INCONSISTENCY

duongkame · 2024-08-21T19:58:34Z

seems to be failing consistently: TestHSync.testUncommittedBlocks:515 expected: but was:

I wonder if it's related to the new commit just merged #7074

Got it. Fixed as @chungen0126 suggested.

jojochuang · 2024-08-21T20:33:02Z

Merged. Thanks @duongkame for the patch and @smengcl @chungen0126 for reviews!

* master: HDDS-11081. Use thread-local instance of FileSystem in Freon tests (#7091) HDDS-11333. Avoid hard-coded current version in upgrade/xcompat tests (#7089) Mark TestPipelineManagerMXBean#testPipelineInfo as flaky Mark TestAddRemoveOzoneManager#testForceBootstrap as flaky HDDS-11352. HDDS-11353. Mark TestOzoneManagerHAWithStoppedNodes as flaky HDDS-11354. Mark TestOzoneManagerSnapshotAcl#testLookupKeyWithNotAllowedUserForPrefixAcl as flaky HDDS-11355. Mark TestMultiBlockWritesWithDnFailures#testMultiBlockWritesWithIntermittentDnFailures as flaky HDDS-11227. Use server default key provider to encrypt/decrypt keys from multiple OMs. (#7081) HDDS-11316. Improve Create Key and Chunk IO Dashboards (#7075) HDDS-11239. Fix KeyOutputStream's exception handling when calling hsync concurrently (#7047)

…an-on-error * HDDS-10239-container-reconciliation: (428 commits) HDDS-11081. Use thread-local instance of FileSystem in Freon tests (apache#7091) HDDS-11333. Avoid hard-coded current version in upgrade/xcompat tests (apache#7089) Mark TestPipelineManagerMXBean#testPipelineInfo as flaky Mark TestAddRemoveOzoneManager#testForceBootstrap as flaky HDDS-11352. HDDS-11353. Mark TestOzoneManagerHAWithStoppedNodes as flaky HDDS-11354. Mark TestOzoneManagerSnapshotAcl#testLookupKeyWithNotAllowedUserForPrefixAcl as flaky HDDS-11355. Mark TestMultiBlockWritesWithDnFailures#testMultiBlockWritesWithIntermittentDnFailures as flaky HDDS-11227. Use server default key provider to encrypt/decrypt keys from multiple OMs. (apache#7081) HDDS-11316. Improve Create Key and Chunk IO Dashboards (apache#7075) HDDS-11239. Fix KeyOutputStream's exception handling when calling hsync concurrently (apache#7047) HDDS-11254. Reconcile commands should be handled by datanode ReplicationSupervisor (apache#7076) HDDS-11331. Fix Datanode unable to report for a long time (apache#7090) HDDS-11346. FS CLI gives incorrect recursive volume deletion prompt (apache#7102) HDDS-11349. Add NullPointer handling when volume/bucket tables are not initialized (apache#7103) HDDS-11209. Avoid insufficient EC pipelines in the container pipeline cache (apache#6974) HDDS-11284. refactor quota repair non-blocking while upgrade (apache#7035) HDDS-9790. Add tests for Overview page (apache#6983) HDDS-10904. [hsync] Enable PutBlock piggybacking and incremental chunk list by default (apache#7074) HDDS-11322. [hsync] Block ECKeyOutputStream from calling hsync and hflush (apache#7098) HDDS-11325. Intermittent failure in TestBlockOutputStreamWithFailures#testContainerClose (apache#7099) ... Conflicts: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainerCheck.java hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OzoneContainer.java

…rrupt-files * HDDS-10239-container-reconciliation: (61 commits) HDDS-11081. Use thread-local instance of FileSystem in Freon tests (apache#7091) HDDS-11333. Avoid hard-coded current version in upgrade/xcompat tests (apache#7089) Mark TestPipelineManagerMXBean#testPipelineInfo as flaky Mark TestAddRemoveOzoneManager#testForceBootstrap as flaky HDDS-11352. HDDS-11353. Mark TestOzoneManagerHAWithStoppedNodes as flaky HDDS-11354. Mark TestOzoneManagerSnapshotAcl#testLookupKeyWithNotAllowedUserForPrefixAcl as flaky HDDS-11355. Mark TestMultiBlockWritesWithDnFailures#testMultiBlockWritesWithIntermittentDnFailures as flaky HDDS-11227. Use server default key provider to encrypt/decrypt keys from multiple OMs. (apache#7081) HDDS-11316. Improve Create Key and Chunk IO Dashboards (apache#7075) HDDS-11239. Fix KeyOutputStream's exception handling when calling hsync concurrently (apache#7047) HDDS-11254. Reconcile commands should be handled by datanode ReplicationSupervisor (apache#7076) HDDS-11331. Fix Datanode unable to report for a long time (apache#7090) HDDS-11346. FS CLI gives incorrect recursive volume deletion prompt (apache#7102) HDDS-11349. Add NullPointer handling when volume/bucket tables are not initialized (apache#7103) HDDS-11209. Avoid insufficient EC pipelines in the container pipeline cache (apache#6974) HDDS-11284. refactor quota repair non-blocking while upgrade (apache#7035) HDDS-9790. Add tests for Overview page (apache#6983) HDDS-10904. [hsync] Enable PutBlock piggybacking and incremental chunk list by default (apache#7074) HDDS-11322. [hsync] Block ECKeyOutputStream from calling hsync and hflush (apache#7098) HDDS-11325. Intermittent failure in TestBlockOutputStreamWithFailures#testContainerClose (apache#7099) ... Conflicts: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java

duongkame force-pushed the HDDS-11239-new branch from d9297a4 to 6120e7e Compare August 8, 2024 20:15

duongkame marked this pull request as ready for review August 8, 2024 23:16

duongkame requested review from jojochuang, szetszwo and smengcl August 8, 2024 23:16

duongkame marked this pull request as draft August 10, 2024 01:27

jojochuang added the hbase HBase on Ozone support label Aug 14, 2024

duongkame marked this pull request as ready for review August 16, 2024 20:03

jojochuang mentioned this pull request Aug 16, 2024

HDDS-11208. Change RatisBlockOutputStream to use HDDS-11174. #7072

Merged

jojochuang reviewed Aug 16, 2024

View reviewed changes

jojochuang reviewed Aug 19, 2024

View reviewed changes

duongkame force-pushed the HDDS-11239-new branch from 9ff2a84 to 7c2441c Compare August 20, 2024 17:00

jojochuang reviewed Aug 20, 2024

View reviewed changes

hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockOutputStream.java Outdated Show resolved Hide resolved

...a/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java Show resolved Hide resolved

smengcl assigned duongkame Aug 20, 2024

jojochuang approved these changes Aug 20, 2024

View reviewed changes

smengcl reviewed Aug 20, 2024

View reviewed changes

...a/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java Outdated Show resolved Hide resolved

chungen0126 reviewed Aug 21, 2024

View reviewed changes

duongkame added 9 commits August 21, 2024 11:27

Add test to verify exception handling when running hsync concurrently.

2ec26ae

HDDS-11239. Fix exception handling when calling hsync concurrently.

dd7615e

New approach to handle retries.

0861983

HDDS-11328. ContainerStateMachine should not crashed because of CHUNK…

eb35873

…_FILE_INCONSISTENCY

Correct wait for trying implementation.

1ce1bcc

Must wait for pending flushes finish before handling exception.

ae9118e

Add synchronized to cleanup, to avoid interfere with write & hsync.

46f4f4e

PR comments.

9560457

Adapt to new code change in master.

a184773

duongkame added 4 commits August 21, 2024 11:27

Make firebug happy.

5fe1f26

Fix TestKeyOutputStream#testConcurrentWriteLimitOne.

f8e0bb8

Finish the sentence.

572eb7a

Fix failing testUncommittedBlocks due to file deletes from other tests.

7a19ccf

duongkame force-pushed the HDDS-11239-new branch from f65df56 to 7a19ccf Compare August 21, 2024 18:30

jojochuang approved these changes Aug 21, 2024

View reviewed changes

jojochuang merged commit 7f47535 into apache:master Aug 21, 2024
39 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HDDS-11239. Fix KeyOutputStream's exception handling when calling hsync concurrently #7047

HDDS-11239. Fix KeyOutputStream's exception handling when calling hsync concurrently #7047

duongkame commented Aug 8, 2024 •

edited

Loading

duongkame commented Aug 16, 2024

jojochuang Aug 16, 2024

jojochuang left a comment

jojochuang commented Aug 20, 2024

duongkame commented Aug 20, 2024

jojochuang left a comment

jojochuang commented Aug 21, 2024

chungen0126 Aug 21, 2024 •

edited

Loading

duongkame commented Aug 21, 2024

jojochuang commented Aug 21, 2024

		@@ -379,7 +380,7 @@ BlockOutputStreamEntry getCurrentStreamEntry() {
		* @return the new current open stream to write to

HDDS-11239. Fix KeyOutputStream's exception handling when calling hsync concurrently #7047

HDDS-11239. Fix KeyOutputStream's exception handling when calling hsync concurrently #7047

Conversation

duongkame commented Aug 8, 2024 • edited Loading

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

duongkame commented Aug 16, 2024

jojochuang Aug 16, 2024

Choose a reason for hiding this comment

jojochuang left a comment

Choose a reason for hiding this comment

jojochuang commented Aug 20, 2024

duongkame commented Aug 20, 2024

jojochuang left a comment

Choose a reason for hiding this comment

jojochuang commented Aug 21, 2024

chungen0126 Aug 21, 2024 • edited Loading

Choose a reason for hiding this comment

duongkame commented Aug 21, 2024

jojochuang commented Aug 21, 2024

duongkame commented Aug 8, 2024 •

edited

Loading

chungen0126 Aug 21, 2024 •

edited

Loading