You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/10/12 14:19:34 UTC
[GitHub] [ozone] sadanand48 opened a new pull request, #3827: HDDS-7316. Print stacktrace to identify the location of RocksObject leaks.
sadanand48 opened a new pull request, #3827:
URL: https://github.com/apache/ozone/pull/3827
## What changes were proposed in this pull request?
Add logging to identify location/trace of unclosed RocksObjects .
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-7316
## How was this patch tested?
Tested on docker with debug logs enabled , identified an instance and fixed in this PR
```
[34mrecon_1 |[0m 2022-10-12 13:56:51 WARN ManagedRocksObjectUtils:49 - RocksIterator is not closed properly
[34mrecon_1 |[0m 2022-10-12 13:56:51 DEBUG ManagedRocksObjectUtils:52 - StackTrace for unclosed instance: org.apache.hadoop.hdds.utils.db.managed.ManagedObject.<init>(ManagedObject.java:34)
[34mrecon_1 |[0m org.apache.hadoop.hdds.utils.db.managed.ManagedRocksIterator.<init>(ManagedRocksIterator.java:28)
[34mrecon_1 |[0m org.apache.hadoop.hdds.utils.db.managed.ManagedRocksIterator.managed(ManagedRocksIterator.java:32)
[34mrecon_1 |[0m org.apache.hadoop.hdds.utils.db.RocksDatabase.newIterator(RocksDatabase.java:492)
[34mrecon_1 |[0m org.apache.hadoop.hdds.utils.db.RDBTable.iterator(RDBTable.java:161)
[34mrecon_1 |[0m org.apache.hadoop.hdds.utils.db.TypedTable.iterator(TypedTable.java:280)
[34mrecon_1 |[0m org.apache.hadoop.ozone.recon.spi.impl.ReconContainerMetadataManagerImpl.initializeKeyContainerTable(ReconContainerMetadataManagerImpl.java:617)
[34mrecon_1 |[0m org.apache.hadoop.ozone.recon.spi.impl.ReconContainerMetadataManagerImpl.initializeTables(ReconContainerMetadataManagerImpl.java:135)
[34mrecon_1 |[0m org.apache.hadoop.ozone.recon.spi.impl.ReconContainerMetadataManagerImpl.<init>(ReconContainerMetadataManagerImpl.java:88)
[34mrecon_1 |[0m org.apache.hadoop.ozone.recon.spi.impl.ReconContainerMetadataManagerImpl$$FastClassByGuice$$7c269485.newInstance(<generated>)
[34mrecon_1 |[0m com.google.inject.internal.cglib.reflect.$FastConstructor.newInstance(FastConstructor.java:40)
[34mrecon_1 |[0m com.google.inject.internal.DefaultConstructionProxyFactory$1.newInstance(DefaultConstructionProxyFactory.java:61)
[34mrecon_1 |[0m com.google.inject.internal.ConstructorInjector.provision(ConstructorInjector.java:105)
[34mrecon_1 |[0m com.google.inject.internal.ConstructorInjector.construct(ConstructorInjector.java:85)
[34mrecon_1 |[0m com.google.inject.internal.ConstructorBindingImpl$Factory.get(ConstructorBindingImpl.java:267)```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai merged pull request #3827: HDDS-7316. Print stacktrace to identify the location of RocksObject leaks.
Posted by GitBox <gi...@apache.org>.
adoroszlai merged PR #3827:
URL: https://github.com/apache/ozone/pull/3827
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sadanand48 commented on a diff in pull request #3827: HDDS-7316. Print stacktrace to identify the location of RocksObject leaks.
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on code in PR #3827:
URL: https://github.com/apache/ozone/pull/3827#discussion_r1004641104
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java:
##########
@@ -33,20 +33,22 @@ private ManagedRocksObjectUtils() {
LoggerFactory.getLogger(ManagedRocksObjectUtils.class);
static void assertClosed(RocksObject rocksObject) {
- ManagedRocksObjectMetrics.INSTANCE.increaseManagedObject();
- if (rocksObject.isOwningHandle()) {
- ManagedRocksObjectMetrics.INSTANCE.increaseLeakObject();
- LOG.warn("{} is not closed properly",
- rocksObject.getClass().getSimpleName());
- }
+ assertClosed(rocksObject, null);
+ }
+
+ public static void assertClosed(ManagedObject<?> object) {
+ assertClosed(object.get(), object.getStackTrace());
}
- static void assertClosed(RocksObject rocksObject, Throwable stack) {
+ static void assertClosed(RocksObject rocksObject, String stackTrace) {
ManagedRocksObjectMetrics.INSTANCE.increaseManagedObject();
if (rocksObject.isOwningHandle()) {
ManagedRocksObjectMetrics.INSTANCE.increaseLeakObject();
LOG.warn("{} is not closed properly",
- rocksObject.getClass().getSimpleName(), stack);
+ rocksObject.getClass().getSimpleName());
+ if (stackTrace != null && LOG.isDebugEnabled()) {
+ LOG.debug("StackTrace for unclosed instance: {}", stackTrace);
+ }
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on a diff in pull request #3827: HDDS-7316. Print stacktrace to identify the location of RocksObject leaks.
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on code in PR #3827:
URL: https://github.com/apache/ozone/pull/3827#discussion_r994986708
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java:
##########
@@ -41,6 +41,20 @@ static void assertClosed(RocksObject rocksObject) {
}
}
+ static void assertClosed(ManagedObject<?> object) {
+ RocksObject rocksObject = object.get();
+ ManagedRocksObjectMetrics.INSTANCE.increaseManagedObject();
+ if (rocksObject.isOwningHandle()) {
+ ManagedRocksObjectMetrics.INSTANCE.increaseLeakObject();
+ LOG.warn("{} is not closed properly",
+ rocksObject.getClass().getSimpleName());
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("StackTrace for unclosed instance: {}",
+ object.getStackTrace());
+ }
+ }
Review Comment:
Thanks @sadanand48 for updating the patch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kerneltime commented on a diff in pull request #3827: HDDS-7316. Print stacktrace to identify the location of RocksObject leaks.
Posted by GitBox <gi...@apache.org>.
kerneltime commented on code in PR #3827:
URL: https://github.com/apache/ozone/pull/3827#discussion_r997600952
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedObject.java:
##########
@@ -42,7 +49,23 @@ public void close() {
@Override
protected void finalize() throws Throwable {
- ManagedRocksObjectUtils.assertClosed(original);
+ ManagedRocksObjectUtils.assertClosed(this);
super.finalize();
}
+
+ public String getStackTrace() {
Review Comment:
Add a unit test for this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kerneltime commented on a diff in pull request #3827: HDDS-7316. Print stacktrace to identify the location of RocksObject leaks.
Posted by GitBox <gi...@apache.org>.
kerneltime commented on code in PR #3827:
URL: https://github.com/apache/ozone/pull/3827#discussion_r997600617
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedObject.java:
##########
@@ -42,7 +49,23 @@ public void close() {
@Override
protected void finalize() throws Throwable {
- ManagedRocksObjectUtils.assertClosed(original);
+ ManagedRocksObjectUtils.assertClosed(this);
super.finalize();
}
+
+ public String getStackTrace() {
+ if (elements != null && elements.length > 0) {
+ // 15 lines should be good enough to know the caller of
+ // the unclosed instance?
+ int maxStackTraceLines = 15;
Review Comment:
This will only show up when something went wrong, it's okay not to limit the stacktrace or use a longer stack trace.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] duongkame commented on a diff in pull request #3827: HDDS-7316. Print stacktrace to identify the location of RocksObject leaks.
Posted by GitBox <gi...@apache.org>.
duongkame commented on code in PR #3827:
URL: https://github.com/apache/ozone/pull/3827#discussion_r997289200
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedObject.java:
##########
@@ -27,8 +29,17 @@
class ManagedObject<T extends RocksObject> implements AutoCloseable {
private final T original;
+ private final StackTraceElement[] elements;
+
+ public static final Logger LOG = LoggerFactory.getLogger(ManagedObject.class);
+
ManagedObject(T original) {
this.original = original;
+ if (LOG.isDebugEnabled()) {
Review Comment:
nit: This separates the decision of creating and logging stacktrace to 2 different authorities/configs. I guess ManagedRocksObjectUtils can export some method like `isStacktraceDebugEnabled` and it can be the only authority of determining if stacktrace should be created and logged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on a diff in pull request #3827: HDDS-7316. Print stacktrace to identify the location of RocksObject leaks.
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on code in PR #3827:
URL: https://github.com/apache/ozone/pull/3827#discussion_r997305850
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedObject.java:
##########
@@ -27,8 +29,17 @@
class ManagedObject<T extends RocksObject> implements AutoCloseable {
private final T original;
+ private final StackTraceElement[] elements;
+
+ public static final Logger LOG = LoggerFactory.getLogger(ManagedObject.class);
+
ManagedObject(T original) {
this.original = original;
+ if (LOG.isDebugEnabled()) {
Review Comment:
`ManagedRocksObjectUtils#LOG` is public, we can also just use it in both places.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sadanand48 commented on a diff in pull request #3827: HDDS-7316. Print stacktrace to identify the location of RocksObject leaks.
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on code in PR #3827:
URL: https://github.com/apache/ozone/pull/3827#discussion_r994234354
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java:
##########
@@ -41,6 +41,20 @@ static void assertClosed(RocksObject rocksObject) {
}
}
+ static void assertClosed(ManagedObject<?> object) {
+ RocksObject rocksObject = object.get();
+ ManagedRocksObjectMetrics.INSTANCE.increaseManagedObject();
+ if (rocksObject.isOwningHandle()) {
+ ManagedRocksObjectMetrics.INSTANCE.increaseLeakObject();
+ LOG.warn("{} is not closed properly",
+ rocksObject.getClass().getSimpleName());
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("StackTrace for unclosed instance: {}",
+ object.getStackTrace());
+ }
+ }
Review Comment:
Done. There were no callers for `assertClosed(RocksObject, Throwable)`, removed it, maybe we can add later if it's of use.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on pull request #3827: HDDS-7316. Print stacktrace to identify the location of RocksObject leaks.
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on PR #3827:
URL: https://github.com/apache/ozone/pull/3827#issuecomment-1293696065
Thanks @sadanand48 for the patch, @duongkame, @kerneltime for the review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on a diff in pull request #3827: HDDS-7316. Print stacktrace to identify the location of RocksObject leaks.
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on code in PR #3827:
URL: https://github.com/apache/ozone/pull/3827#discussion_r998531220
##########
hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestRDBStoreIterator.java:
##########
@@ -293,4 +298,28 @@ public void testNormalPrefixedIterator() throws IOException {
iter.close();
}
+
+ @Test
+ public void testGetStackTrace() {
+ ManagedRocksIterator iterator = mock(ManagedRocksIterator.class);
+ RocksIterator mock = mock(RocksIterator.class);
+ when(iterator.get()).thenReturn(mock);
+ when(mock.isOwningHandle()).thenReturn(true);
+ ManagedRocksObjectUtils.assertClosed(iterator);
+ verify(iterator, times(1)).getStackTrace();
Review Comment:
```
M D RV: Return value of org.apache.hadoop.hdds.utils.db.managed.ManagedRocksIterator.getStackTrace() ignored, but method has no side effect At TestRDBStoreIterator.java:[line 309]
```
https://github.com/apache/ozone/actions/runs/3271563470/jobs/5382670610#step:7:10
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on a diff in pull request #3827: HDDS-7316. Print stacktrace to identify the location of RocksObject leaks.
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on code in PR #3827:
URL: https://github.com/apache/ozone/pull/3827#discussion_r995006673
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedObject.java:
##########
@@ -27,8 +27,11 @@
class ManagedObject<T extends RocksObject> implements AutoCloseable {
private final T original;
+ private final StackTraceElement[] elements;
+
ManagedObject(T original) {
this.original = original;
+ this.elements = Thread.currentThread().getStackTrace();
Review Comment:
Good point. If stack trace is used only when debug is enabled, we can limit its creation, too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on a diff in pull request #3827: HDDS-7316. Print stacktrace to identify the location of RocksObject leaks.
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on code in PR #3827:
URL: https://github.com/apache/ozone/pull/3827#discussion_r993756288
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java:
##########
@@ -41,6 +41,20 @@ static void assertClosed(RocksObject rocksObject) {
}
}
+ static void assertClosed(ManagedObject<?> object) {
+ RocksObject rocksObject = object.get();
+ ManagedRocksObjectMetrics.INSTANCE.increaseManagedObject();
+ if (rocksObject.isOwningHandle()) {
+ ManagedRocksObjectMetrics.INSTANCE.increaseLeakObject();
+ LOG.warn("{} is not closed properly",
+ rocksObject.getClass().getSimpleName());
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("StackTrace for unclosed instance: {}",
+ object.getStackTrace());
+ }
+ }
Review Comment:
Would be nice to avoid duplicating logic from `assertClosed(RocksObject, Throwable)`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sadanand48 commented on a diff in pull request #3827: HDDS-7316. Print stacktrace to identify the location of RocksObject leaks.
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on code in PR #3827:
URL: https://github.com/apache/ozone/pull/3827#discussion_r997363785
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedObject.java:
##########
@@ -27,8 +29,17 @@
class ManagedObject<T extends RocksObject> implements AutoCloseable {
private final T original;
+ private final StackTraceElement[] elements;
+
+ public static final Logger LOG = LoggerFactory.getLogger(ManagedObject.class);
+
ManagedObject(T original) {
this.original = original;
+ if (LOG.isDebugEnabled()) {
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on a diff in pull request #3827: HDDS-7316. Print stacktrace to identify the location of RocksObject leaks.
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on code in PR #3827:
URL: https://github.com/apache/ozone/pull/3827#discussion_r1001059048
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java:
##########
@@ -33,20 +33,22 @@ private ManagedRocksObjectUtils() {
LoggerFactory.getLogger(ManagedRocksObjectUtils.class);
static void assertClosed(RocksObject rocksObject) {
- ManagedRocksObjectMetrics.INSTANCE.increaseManagedObject();
- if (rocksObject.isOwningHandle()) {
- ManagedRocksObjectMetrics.INSTANCE.increaseLeakObject();
- LOG.warn("{} is not closed properly",
- rocksObject.getClass().getSimpleName());
- }
+ assertClosed(rocksObject, null);
+ }
+
+ public static void assertClosed(ManagedObject<?> object) {
+ assertClosed(object.get(), object.getStackTrace());
}
- static void assertClosed(RocksObject rocksObject, Throwable stack) {
+ static void assertClosed(RocksObject rocksObject, String stackTrace) {
ManagedRocksObjectMetrics.INSTANCE.increaseManagedObject();
if (rocksObject.isOwningHandle()) {
ManagedRocksObjectMetrics.INSTANCE.increaseLeakObject();
LOG.warn("{} is not closed properly",
- rocksObject.getClass().getSimpleName(), stack);
+ rocksObject.getClass().getSimpleName());
+ if (stackTrace != null && LOG.isDebugEnabled()) {
+ LOG.debug("StackTrace for unclosed instance: {}", stackTrace);
+ }
Review Comment:
Nit: better execute as a single log statement for each case to ensure that the stack trace and object class are associated.
In other words: drop `LOG.debug`, but change content of warning depending on `stackTrace != null && LOG.isDebugEnabled()`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sadanand48 commented on a diff in pull request #3827: HDDS-7316. Print stacktrace to identify the location of RocksObject leaks.
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on code in PR #3827:
URL: https://github.com/apache/ozone/pull/3827#discussion_r997868359
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedObject.java:
##########
@@ -42,7 +49,23 @@ public void close() {
@Override
protected void finalize() throws Throwable {
- ManagedRocksObjectUtils.assertClosed(original);
+ ManagedRocksObjectUtils.assertClosed(this);
super.finalize();
}
+
+ public String getStackTrace() {
+ if (elements != null && elements.length > 0) {
+ // 15 lines should be good enough to know the caller of
+ // the unclosed instance?
+ int maxStackTraceLines = 15;
Review Comment:
Ok , I added this as the stackTrace caught in this example was approximately 50 lines and was occupying the entire screen and if there are multiple such instances it can flood the debug log with these but I guess we should be okay if there are not many such instances.
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedObject.java:
##########
@@ -42,7 +49,23 @@ public void close() {
@Override
protected void finalize() throws Throwable {
- ManagedRocksObjectUtils.assertClosed(original);
+ ManagedRocksObjectUtils.assertClosed(this);
super.finalize();
}
+
+ public String getStackTrace() {
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on pull request #3827: HDDS-7316. Print stacktrace to identify the location of RocksObject leaks.
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on PR #3827:
URL: https://github.com/apache/ozone/pull/3827#issuecomment-1278018192
@duongkame please review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] duongkame commented on a diff in pull request #3827: HDDS-7316. Print stacktrace to identify the location of RocksObject leaks.
Posted by GitBox <gi...@apache.org>.
duongkame commented on code in PR #3827:
URL: https://github.com/apache/ozone/pull/3827#discussion_r994999431
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedObject.java:
##########
@@ -27,8 +27,11 @@
class ManagedObject<T extends RocksObject> implements AutoCloseable {
private final T original;
+ private final StackTraceElement[] elements;
+
ManagedObject(T original) {
this.original = original;
+ this.elements = Thread.currentThread().getStackTrace();
Review Comment:
I'm not sure it's a good idea to pay the cost of capturing the current stacktrace for every allocated RockObject in production. Maybe we should do that for debugging purpose only (when `LOG.isDebugEnabled()`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org