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.

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

HDDS-4327. Potential resource leakage using BatchOperation. #1493

Merged
merged 3 commits into from
Oct 14, 2020

Conversation

bharatviswa504
Copy link
Contributor

What changes were proposed in this pull request?

Potential resource leakage using BatchOperation.

Use try enclosed resource/close the batch once after it usage is completed.

What is the link to the Apache JIRA

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

How was this patch tested?

Existing tests.

Copy link
Contributor

@hanishakoneru hanishakoneru left a comment

Choose a reason for hiding this comment

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

Thanks @bharatviswa504 for fixing this.
Have one question (posted inline). LGTM otherwise.

for (Map.Entry<Long, Long> entry : deleteTransactionMap.entrySet()) {
try(org.apache.hadoop.hdds.utils.db.BatchOperation batchOperation =
batchHandler.initBatchOperation()) {
lock.lock();
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't lock() be called outside the try block? If there is a chance that the initBatchOperation() can fail, then lock.unlock() would throw IllegalMonitorStateException as the lock is not held by it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Fixed in the latest commit.

@hanishakoneru
Copy link
Contributor

Thanks @bharatviswa504. +1 for merge.

@hanishakoneru hanishakoneru merged commit 342bf6d into apache:master Oct 14, 2020
errose28 added a commit to errose28/ozone that referenced this pull request Oct 19, 2020
* master:
  HDDS-4301. SCM CA certificate does not encode KeyUsage extension properly (apache#1468)
  HDDS-4158. Provide a class type for Java based configuration (apache#1407)
  HDDS-4297. Allow multiple transactions per container to be sent for deletion by SCM.
  HDDS-2922. Balance ratis leader distribution in datanodes (apache#1371)
  HDDS-4269. Ozone DataNode thinks a volume is failed if an unexpected file is in the HDDS root directory. (apache#1490)
  HDDS-4327. Potential resource leakage using BatchOperation. (apache#1493)
  HDDS-3995. Fix s3g met NPE exception while write file by multiPartUpload (apache#1499)
  HDDS-4343. ReplicationManager.handleOverReplicatedContainer() does not handle unhealthyReplicas properly. (apache#1495)
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.

2 participants