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-12195. Implement skip() in OzoneFSInputStream #7801

Merged
merged 2 commits into from
Feb 5, 2025
Merged

HDDS-12195. Implement skip() in OzoneFSInputStream #7801

merged 2 commits into from
Feb 5, 2025

Conversation

oneonestar
Copy link
Contributor

What changes were proposed in this pull request?

HDDS-12195. Implement skip() in OzoneFSInputStream

Please describe your PR in detail:

  • The skip() method is not implemented in OzoneFSInputStream. As a result, it defaults to java.io.InputStream#skip, which uses read() and slows down performance.
  • Implement skip() in OzoneFSInputStream to delegate skip() to inputStream

What is the link to the Apache JIRA

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

How was this patch tested?

Integration Tests

@adoroszlai
Copy link
Contributor

Thanks @oneonestar for the patch. Please enable the build-branch workflow in your fork.

@adoroszlai adoroszlai requested a review from jojochuang February 4, 2025 05:45
@adoroszlai
Copy link
Contributor

testOfsHSync and testO3fsHSync seems to fail consistently:

TestHSync.testO3fsHSync(boolean)[2]  Time elapsed: 0.043 s  <<< FAILURE!
AssertionFailedError: expected: <32> but was: <1>
	...
	at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:632)
	at org.apache.hadoop.fs.ozone.TestHSync.lambda$runTestHSync$8(TestHSync.java:817)
	at org.apache.ozone.test.GenericTestUtils.waitFor(GenericTestUtils.java:214)
	at org.apache.hadoop.fs.ozone.TestHSync.runTestHSync(TestHSync.java:812)
	at org.apache.hadoop.fs.ozone.TestHSync.runTestHSync(TestHSync.java:766)
	at org.apache.hadoop.fs.ozone.TestHSync.testO3fsHSync(TestHSync.java:474)

@oneonestar
Copy link
Contributor Author

Looking into that. Might need sometime to understand the real cause.

image
Seems like the InputStream only have 1 byte and caused the following line to only skip 1.

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

Line 227 in cfe56de

long toSkip = Math.min(n, length - getPos());

@oneonestar
Copy link
Contributor Author

I created another jira and PR to fix the underlying cause:

@jojochuang jojochuang mentioned this pull request Feb 4, 2025
Merged
jojochuang
jojochuang reviewed Feb 4, 2025
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.

LGTM +1 pending a successful precommit test result.
No need for new tests since test case already covers it, as I mentioned in #7809.

@adoroszlai adoroszlai merged commit 182bd53 into apache:master Feb 5, 2025
42 checks passed
@adoroszlai
Copy link
Contributor

Thanks @oneonestar for the patch, @jojochuang for the review.

@oneonestar oneonestar deleted the star/impl_skip branch February 5, 2025 12:09
errose28 added a commit to errose28/ozone that referenced this pull request Feb 5, 2025
@errose28
Merge branch 'master' into HDDS-10239-container-reconciliation
3797c28 
* master: (168 commits)
  HDDS-12112. Fix interval used for Chunk Read/Write Dashboard (apache#7724)
  HDDS-12212. Fix grammar in decommissioning and observability documentation (apache#7815)
  HDDS-12195. Implement skip() in OzoneFSInputStream (apache#7801)
  HDDS-12200. Fix grammar in OM HA, EC and Snapshot doc (apache#7806)
  HDDS-12202. OpsCreate and OpsAppend metrics not incremented (apache#7811)
  HDDS-12203. Initialize block length before skip (apache#7809)
  HDDS-12183. Reuse cluster across safe test classes (apache#7793)
  HDDS-11714. resetDeletedBlockRetryCount with --all may fail and can cause long db lock in large cluster. (apache#7665)
  HDDS-12186. (addendum) Avoid array allocation for table iterator (apache#7799)
  HDDS-12186. Avoid array allocation for table iterator. (apache#7797)
  HDDS-11508. Decouple delete batch limits from Ratis request size for DirectoryDeletingService. (apache#7365)
  HDDS-12073. Don't show Source Bucket and Volume if null in DU metadata (apache#7760)
  HDDS-12142. Save logs from build check (apache#7782)
  HDDS-12163. Reduce number of individual getCapacity/getAvailable/getUsedSpace calls (apache#7790)
  HDDS-12176. Trivial dependency cleanup.(apache#7787)
  HDDS-12181. Bump jline to 3.29.0 (apache#7789)
  HDDS-12165. Refactor VolumeInfoMetrics to use getCurrentUsage (apache#7784)
  HDDS-12085. Add manual refresh button for DU page (apache#7780)
  HDDS-12132. Parameterize testUpdateTransactionInfoTable for SCM (apache#7768)
  HDDS-11277. Remove dependency on hadoop-hdfs in Ozone client (apache#7781)
  ...

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/keyvalue/KeyValueHandler.java
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java
hadoop-ozone/dist/src/main/smoketest/admincli/container.robot
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/ClosedContainerReplicator.java
errose28 added a commit to errose28/ozone that referenced this pull request Feb 6, 2025
@errose28
Merge branch 'master' into HDDS-10239-container-reconciliation
2cfd22a 
* master: (168 commits)
  HDDS-12112. Fix interval used for Chunk Read/Write Dashboard (apache#7724)
  HDDS-12212. Fix grammar in decommissioning and observability documentation (apache#7815)
  HDDS-12195. Implement skip() in OzoneFSInputStream (apache#7801)
  HDDS-12200. Fix grammar in OM HA, EC and Snapshot doc (apache#7806)
  HDDS-12202. OpsCreate and OpsAppend metrics not incremented (apache#7811)
  HDDS-12203. Initialize block length before skip (apache#7809)
  HDDS-12183. Reuse cluster across safe test classes (apache#7793)
  HDDS-11714. resetDeletedBlockRetryCount with --all may fail and can cause long db lock in large cluster. (apache#7665)
  HDDS-12186. (addendum) Avoid array allocation for table iterator (apache#7799)
  HDDS-12186. Avoid array allocation for table iterator. (apache#7797)
  HDDS-11508. Decouple delete batch limits from Ratis request size for DirectoryDeletingService. (apache#7365)
  HDDS-12073. Don't show Source Bucket and Volume if null in DU metadata (apache#7760)
  HDDS-12142. Save logs from build check (apache#7782)
  HDDS-12163. Reduce number of individual getCapacity/getAvailable/getUsedSpace calls (apache#7790)
  HDDS-12176. Trivial dependency cleanup.(apache#7787)
  HDDS-12181. Bump jline to 3.29.0 (apache#7789)
  HDDS-12165. Refactor VolumeInfoMetrics to use getCurrentUsage (apache#7784)
  HDDS-12085. Add manual refresh button for DU page (apache#7780)
  HDDS-12132. Parameterize testUpdateTransactionInfoTable for SCM (apache#7768)
  HDDS-11277. Remove dependency on hadoop-hdfs in Ozone client (apache#7781)
  ...

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/keyvalue/KeyValueHandler.java
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java
hadoop-ozone/dist/src/main/smoketest/admincli/container.robot
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/ClosedContainerReplicator.java
errose28 added a commit to errose28/ozone that referenced this pull request Feb 7, 2025
@errose28
Merge branch 'HDDS-10239-container-reconciliation' into HDDS-12079-tr…
dceb3ef 
…ee-improvements

* HDDS-10239-container-reconciliation: (168 commits)
  HDDS-12112. Fix interval used for Chunk Read/Write Dashboard (apache#7724)
  HDDS-12212. Fix grammar in decommissioning and observability documentation (apache#7815)
  HDDS-12195. Implement skip() in OzoneFSInputStream (apache#7801)
  HDDS-12200. Fix grammar in OM HA, EC and Snapshot doc (apache#7806)
  HDDS-12202. OpsCreate and OpsAppend metrics not incremented (apache#7811)
  HDDS-12203. Initialize block length before skip (apache#7809)
  HDDS-12183. Reuse cluster across safe test classes (apache#7793)
  HDDS-11714. resetDeletedBlockRetryCount with --all may fail and can cause long db lock in large cluster. (apache#7665)
  HDDS-12186. (addendum) Avoid array allocation for table iterator (apache#7799)
  HDDS-12186. Avoid array allocation for table iterator. (apache#7797)
  HDDS-11508. Decouple delete batch limits from Ratis request size for DirectoryDeletingService. (apache#7365)
  HDDS-12073. Don't show Source Bucket and Volume if null in DU metadata (apache#7760)
  HDDS-12142. Save logs from build check (apache#7782)
  HDDS-12163. Reduce number of individual getCapacity/getAvailable/getUsedSpace calls (apache#7790)
  HDDS-12176. Trivial dependency cleanup.(apache#7787)
  HDDS-12181. Bump jline to 3.29.0 (apache#7789)
  HDDS-12165. Refactor VolumeInfoMetrics to use getCurrentUsage (apache#7784)
  HDDS-12085. Add manual refresh button for DU page (apache#7780)
  HDDS-12132. Parameterize testUpdateTransactionInfoTable for SCM (apache#7768)
  HDDS-11277. Remove dependency on hadoop-hdfs in Ozone client (apache#7781)
  ...
nandakumar131 pushed a commit to nandakumar131/ozone that referenced this pull request Feb 10, 2025
@oneonestar @nandakumar131
HDDS-12195. Implement skip() in OzoneFSInputStream (apache#7801)
f7deae1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jojochuang jojochuang jojochuang left review comments

@adoroszlai adoroszlai adoroszlai 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.
3 participants
@oneonestar @adoroszlai @jojochuang