Skip to content
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.

Sign up for GitHub

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

Jump to bottom

HDDS-11340. Avoid extra PubBlock call when a full block is closed #7094

Merged
merged 2 commits into from
Aug 20, 2024
Merged

HDDS-11340. Avoid extra PubBlock call when a full block is closed #7094

merged 2 commits into from
Aug 20, 2024

Conversation

ChenSammi
Copy link
Contributor

@ChenSammi ChenSammi commented Aug 19, 2024 •
edited
Loading

What changes were proposed in this pull request?

Currently, for every client side buffer flush size(32MB default), there will be a putBlock call to DN to persist the block metadata. For a 128MB block, there will be 4 putBlock calls during the data write, and one extra putBlock at the end when this block is closed due to full. This last putBlock will add the flag "eof" in the request to indicate that this block is completed.
This task aims to remove this last putBlock call in case this block is a full block when block close is called.

What is the link to the Apache JIRA

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

How was this patch tested?

Existing test cases, plus manual audit log verification. Before the patch, here are the audit logs, the block size 32MB.

2024-08-19 21:29:26,396 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2001 locID: 113750153625602004 bcsId: 0 replicaIndex: 0, offset=0, size=4194304, stage=WRITE_DATA} | ret=null | perf={preOpLatencyMs=19, opLatencyMs=32} |
2024-08-19 21:29:26,397 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2001 locID: 113750153625602004 bcsId: 0 replicaIndex: 0, offset=4194304, size=4194304, stage=WRITE_DATA} | ret=null | perf={preOpLatencyMs=0, opLatencyMs=1} |  
2024-08-19 21:29:26,418 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2001 locID: 113750153625602004 bcsId: 0 replicaIndex: 0, offset=0, size=4194304, stage=COMMIT_DATA} | ret=null | perf={preOpLatencyMs=0, opLatencyMs=0} |  
2024-08-19 21:29:26,422 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2001 locID: 113750153625602004 bcsId: 0 replicaIndex: 0, offset=4194304, size=4194304, stage=COMMIT_DATA} | ret=null | perf={preOpLatencyMs=0, opLatencyMs=0} |
2024-08-19 21:29:26,436 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2001 locID: 113750153625602004 bcsId: 0 replicaIndex: 0, offset=8388608, size=4194304, stage=WRITE_DATA} | ret=null | perf={preOpLatencyMs=0, opLatencyMs=2} |
2024-08-19 21:29:26,439 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2001 locID: 113750153625602004 bcsId: 0 replicaIndex: 0, offset=8388608, size=4194304, stage=COMMIT_DATA} | ret=null | perf={preOpLatencyMs=0, opLatencyMs=0} |
2024-08-19 21:29:26,465 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2001 locID: 113750153625602004 bcsId: 0 replicaIndex: 0, offset=12582912, size=4194304, stage=WRITE_DATA} | ret=null | perf={preOpLatencyMs=0, opLatencyMs=1} |
2024-08-19 21:29:26,467 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2001 locID: 113750153625602004 bcsId: 0 replicaIndex: 0, offset=12582912, size=4194304, stage=COMMIT_DATA} | ret=null | perf={preOpLatencyMs=0, opLatencyMs=0} |
2024-08-19 21:29:26,477 | INFO  | DNAudit | user=null | ip=null | op=PUT_BLOCK {blockData=[blockId=conID: 2001 locID: 113750153625602004 bcsId: 0 replicaIndex: null, size=16777216]} | ret=null | perf={preOpLatencyMs=0, opLatencyMs=7} |
2024-08-19 21:29:26,483 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2001 locID: 113750153625602004 bcsId: 0 replicaIndex: 0, offset=16777216, size=4194304, stage=WRITE_DATA} | ret=null | perf={preOpLatencyMs=0, opLatencyMs=2} |
2024-08-19 21:29:26,484 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2001 locID: 113750153625602004 bcsId: 0 replicaIndex: 0, offset=16777216, size=4194304, stage=COMMIT_DATA} | ret=null | perf={preOpLatencyMs=0, opLatencyMs=0} |
2024-08-19 21:29:26,500 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2001 locID: 113750153625602004 bcsId: 0 replicaIndex: 0, offset=20971520, size=4194304, stage=WRITE_DATA} | ret=null | perf={preOpLatencyMs=0, opLatencyMs=1} |
2024-08-19 21:29:26,504 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2001 locID: 113750153625602004 bcsId: 0 replicaIndex: 0, offset=20971520, size=4194304, stage=COMMIT_DATA} | ret=null | perf={preOpLatencyMs=0, opLatencyMs=0} |
2024-08-19 21:29:26,524 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2001 locID: 113750153625602004 bcsId: 0 replicaIndex: 0, offset=25165824, size=4194304, stage=WRITE_DATA} | ret=null | perf={preOpLatencyMs=0, opLatencyMs=1} |
2024-08-19 21:29:26,526 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2001 locID: 113750153625602004 bcsId: 0 replicaIndex: 0, offset=25165824, size=4194304, stage=COMMIT_DATA} | ret=null | perf={preOpLatencyMs=0, opLatencyMs=0} |
2024-08-19 21:29:26,539 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2001 locID: 113750153625602004 bcsId: 0 replicaIndex: 0, offset=29360128, size=4194304, stage=WRITE_DATA} | ret=null | perf={preOpLatencyMs=0, opLatencyMs=1} |
2024-08-19 21:29:26,545 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2001 locID: 113750153625602004 bcsId: 0 replicaIndex: 0, offset=29360128, size=4194304, stage=COMMIT_DATA} | ret=null | perf={preOpLatencyMs=0, opLatencyMs=0} |
2024-08-19 21:29:26,551 | INFO  | DNAudit | user=null | ip=null | op=PUT_BLOCK {blockData=[blockId=conID: 2001 locID: 113750153625602004 bcsId: 0 replicaIndex: null, size=33554432]} | ret=null | perf={preOpLatencyMs=0, opLatencyMs=2} |
2024-08-19 21:29:26,576 | INFO  | DNAudit | user=null | ip=null | op=PUT_BLOCK {blockData=[blockId=conID: 2001 locID: 113750153625602004 bcsId: 0 replicaIndex: null, size=33554432]} | ret=null | perf={preOpLatencyMs=0, opLatencyMs=4} |

after the patch

2024-08-19 22:08:28,508 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2004 locID: 113750153625602007 bcsId: 0 replicaIndex: 0, blockDataSize=4194304} | ret=null | perf={preOpLatencyMs=-561240420885044, opLatencyMs=28845125} |
2024-08-19 22:08:28,512 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2004 locID: 113750153625602007 bcsId: 0 replicaIndex: 0, blockDataSize=4194304} | ret=null | perf={preOpLatencyMs=-561240452817194, opLatencyMs=1545333} |
2024-08-19 22:08:28,514 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2004 locID: 113750153625602007 bcsId: 0 replicaIndex: 0, blockDataSize=4194304} | ret=null | perf={preOpLatencyMs=-561240454482485, opLatencyMs=2044583} |
2024-08-19 22:08:28,522 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2004 locID: 113750153625602007 bcsId: 0 replicaIndex: 0, blockDataSize=4194304} | ret=null | perf={preOpLatencyMs=-561240462158935, opLatencyMs=1801875} |
2024-08-19 22:08:28,522 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2004 locID: 113750153625602007 bcsId: 0 replicaIndex: 0, blockDataSize=4194304} | ret=null | perf={preOpLatencyMs=-561240464377266, opLatencyMs=52292} |
2024-08-19 22:08:28,525 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2004 locID: 113750153625602007 bcsId: 0 replicaIndex: 0, blockDataSize=4194304} | ret=null | perf={preOpLatencyMs=-561240467488055, opLatencyMs=61125} |
2024-08-19 22:08:28,526 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2004 locID: 113750153625602007 bcsId: 0 replicaIndex: 0, blockDataSize=4194304} | ret=null | perf={preOpLatencyMs=-561240468259054, opLatencyMs=86708} |
2024-08-19 22:08:28,527 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2004 locID: 113750153625602007 bcsId: 0 replicaIndex: 0, blockDataSize=4194304} | ret=null | perf={preOpLatencyMs=-561240468926345, opLatencyMs=34291} |
2024-08-19 22:08:28,537 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2004 locID: 113750153625602007 bcsId: 0 replicaIndex: 0, blockDataSize=4194304} | ret=null | perf={preOpLatencyMs=-561240476854462, opLatencyMs=2298291} |
2024-08-19 22:08:28,538 | INFO  | DNAudit | user=null | ip=null | op=PUT_BLOCK {blockData=[blockId=conID: 2004 locID: 113750153625602007 bcsId: 0 replicaIndex: null, size=16777216]} | ret=null | perf={preOpLatencyMs=-561240470193760, opLatencyMs=9853542} |
2024-08-19 22:08:28,538 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2004 locID: 113750153625602007 bcsId: 0 replicaIndex: 0, blockDataSize=4194304} | ret=null | perf={preOpLatencyMs=-561240480741874, opLatencyMs=54167} |
2024-08-19 22:08:28,551 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2004 locID: 113750153625602007 bcsId: 0 replicaIndex: 0, blockDataSize=4194304} | ret=null | perf={preOpLatencyMs=-561240491555406, opLatencyMs=1393083} |
2024-08-19 22:08:28,552 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2004 locID: 113750153625602007 bcsId: 0 replicaIndex: 0, blockDataSize=4194304} | ret=null | perf={preOpLatencyMs=-561240494505445, opLatencyMs=62625} |
2024-08-19 22:08:28,566 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2004 locID: 113750153625602007 bcsId: 0 replicaIndex: 0, blockDataSize=4194304} | ret=null | perf={preOpLatencyMs=-561240506190266, opLatencyMs=1912792} |
2024-08-19 22:08:28,568 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2004 locID: 113750153625602007 bcsId: 0 replicaIndex: 0, blockDataSize=4194304} | ret=null | perf={preOpLatencyMs=-561240510633054, opLatencyMs=72541} |
2024-08-19 22:08:28,578 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2004 locID: 113750153625602007 bcsId: 0 replicaIndex: 0, blockDataSize=4194304} | ret=null | perf={preOpLatencyMs=-561240518328587, opLatencyMs=1434459} |
2024-08-19 22:08:28,579 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2004 locID: 113750153625602007 bcsId: 0 replicaIndex: 0, blockDataSize=4194304} | ret=null | perf={preOpLatencyMs=-561240521221084, opLatencyMs=61167} |
2024-08-19 22:08:28,581 | INFO  | DNAudit | user=null | ip=null | op=PUT_BLOCK {blockData=[blockId=conID: 2004 locID: 113750153625602007 bcsId: 0 replicaIndex: null, size=33554432]} | ret=null | perf={preOpLatencyMs=-561240521807333, opLatencyMs=1738125} |
2024-08-19 22:08:28,613 | INFO  | DNAudit | user=null | ip=null | op=WRITE_CHUNK {blockData=conID: 2005 locID: 113750153625602008 bcsId: 0 replicaIndex: 0, blockDataSize=4194304} | ret=null | perf={preOpLatencyMs=-561240549952095, opLatencyMs=5258083} |

jojochuang
jojochuang reviewed Aug 19, 2024
View reviewed changes
Copy link
Contributor

@jojochuang jojochuang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

jojochuang
jojochuang reviewed Aug 19, 2024
View reviewed changes
Copy link
Contributor

@jojochuang jojochuang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR makes sense to me except for the findbug error.

jojochuang
jojochuang approved these changes Aug 20, 2024
View reviewed changes
Copy link
Contributor

@jojochuang jojochuang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@jojochuang jojochuang merged commit 93ee77d into apache:master Aug 20, 2024
39 checks passed
@jojochuang
Copy link
Contributor

Merged. Thanks @ChenSammi

@ChenSammi
Copy link
Contributor Author

Thanks @jojochuang for the review.

errose28 added a commit to errose28/ozone that referenced this pull request Aug 21, 2024
@errose28
Merge branch 'master' into HDDS-10239-container-reconciliation
7186a77 
* master: (50 commits)
  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)
  HDDS-11340. Avoid extra PubBlock call when a full block is closed (apache#7094)
  HDDS-11155. Improve Volumes page UI (apache#7048)
  HDDS-11324. Negative value preOpLatencyMs in DN audit log (apache#7093)
  HDDS-11246. [Recon] Use optional chaining instead of explicit undefined check for Objects in Container and Pipeline Module. (apache#7037)
  HDDS-11323. Mark TestLeaseRecovery as flaky
  HDDS-11338. Bump zstd-jni to 1.5.6-4 (apache#7085)
  HDDS-11337. Bump Spring Framework to 5.3.39 (apache#7084)
  HDDS-11327. [hsync] Revert config default ozone.fs.hsync.enabled to false (apache#7079)
  HDDS-11325. Mark testWriteMoreThanMaxFlushSize as flaky
  HDDS-11336. Bump slf4j to 2.0.16 (apache#7086)
  HDDS-11335. Bump exec-maven-plugin to 3.4.1 (apache#7087)
  ...

Conflicts:
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeConfiguration.java
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OzoneContainer.java
errose28 added a commit to errose28/ozone that referenced this pull request Aug 21, 2024
@errose28
Merge branch 'master' into HDDS-10239-container-reconciliation
3355701 
* master: (50 commits)
  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)
  HDDS-11340. Avoid extra PubBlock call when a full block is closed (apache#7094)
  HDDS-11155. Improve Volumes page UI (apache#7048)
  HDDS-11324. Negative value preOpLatencyMs in DN audit log (apache#7093)
  HDDS-11246. [Recon] Use optional chaining instead of explicit undefined check for Objects in Container and Pipeline Module. (apache#7037)
  HDDS-11323. Mark TestLeaseRecovery as flaky
  HDDS-11338. Bump zstd-jni to 1.5.6-4 (apache#7085)
  HDDS-11337. Bump Spring Framework to 5.3.39 (apache#7084)
  HDDS-11327. [hsync] Revert config default ozone.fs.hsync.enabled to false (apache#7079)
  HDDS-11325. Mark testWriteMoreThanMaxFlushSize as flaky
  HDDS-11336. Bump slf4j to 2.0.16 (apache#7086)
  HDDS-11335. Bump exec-maven-plugin to 3.4.1 (apache#7087)
  ...

Conflicts:
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeConfiguration.java
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OzoneContainer.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jojochuang jojochuang jojochuang approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ChenSammi @jojochuang