Skip to content

Conversation

@szetszwo
Copy link
Contributor

What changes were proposed in this pull request?

All the implementations of the Table.close() method are noop. We should remove it.

The patch of this PR is quite big due to the try-with-resource change for auto-closing Table.

What is the link to the Apache JIRA

HDDS-13294

How was this patch tested?

By existing tests.

Copy link
Contributor

@sarvekshayr sarvekshayr left a comment

Choose a reason for hiding this comment

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

Thanks for the patch @szetszwo. LGTM

@szetszwo szetszwo requested a review from adoroszlai June 19, 2025 15:24
@adoroszlai adoroszlai marked this pull request as draft June 20, 2025 10:00
@adoroszlai adoroszlai marked this pull request as ready for review June 20, 2025 11:13
Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

Thanks @szetszwo for the patch.

Comment on lines 308 to 312
try {
if (pipelineStore != null) {
pipelineStore.close();
pipelineStore = null;
}
} catch (Exception ex) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: try-catch and if no longer needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually want to remove the close() method. Unfortunately, it is not easy to do.

@szetszwo szetszwo merged commit db6ed84 into apache:master Jun 20, 2025
79 of 80 checks passed
@szetszwo
Copy link
Contributor Author

Thanks @sarvekshayr and @adoroszlai for reviewing this!

aswinshakil added a commit to aswinshakil/ozone that referenced this pull request Jun 23, 2025
…239-container-reconciliation

Commits: 9

05567e6 HDDS-13317. Table should support empty array/String (apache#8676)
803fa31 HDDS-13313. Bump maven-javadoc-plugin to 3.11.2 (apache#8672)
774b3ee HDDS-13315. Bump log4j2 to 2.25.0 (apache#8670)
b86fad8 HDDS-13316. Bump commons-text to 1.13.1 (apache#8669)
c043be7 HDDS-13312. Bump maven-patch-plugin to 1.3 (apache#8673)
db6ed84 HDDS-13294. Remove the Table.close() method. (apache#8658)
e1365a7 HDDS-13286. Fail stream write when the volume is full.  (apache#8644)
af3a0f0 HDDS-13307. integration (flaky) fails with all tests passing (apache#8667)
d207c18 HDDS-13301. Fix TestSchemaOneBackwardsCompatibility (apache#8666)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants