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/08/23 18:25:55 UTC
[GitHub] [ozone] duongnguyen0 opened a new pull request, #3713: HDDS-7166. Memory leak in Recon when replacing DB for new checkpoint
duongnguyen0 opened a new pull request, #3713:
URL: https://github.com/apache/ozone/pull/3713
## What changes were proposed in this pull request?
Found this when baking RocksDB changes in our internal test cluster.
When updating the database object for a new checkpoint, the existing DB is replaced without closing.
https://issues.apache.org/jira/browse/HDDS-7166
## How was this patch tested?
Unit test, standard CI.
--
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 #3713: HDDS-7166. Memory leak in Recon when replacing DB for new checkpoint
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on PR #3713:
URL: https://github.com/apache/ozone/pull/3713#issuecomment-1226979988
Please make sure all tests pass before merging.
--
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 pull request #3713: HDDS-7166. Memory leak in Recon when replacing DB for new checkpoint
Posted by GitBox <gi...@apache.org>.
kerneltime commented on PR #3713:
URL: https://github.com/apache/ozone/pull/3713#issuecomment-1226599055
> > Should the logic to close the DB be moved inside `initializeNewRdbStore`?
>
> Thanks for the review, @kerneltime. I guess the closure of the existing DB should be in `updateOmDB(File newDbLocation)` as it's meant to replace the DB object. But I'm ok either way, think it can be better if `initializeNewRdbStore` only creates and initializes `DBStore` and doesn't attempt to `set`.
I agree. It would be good to place the new DB, initialize it, and then make it available. Looking at the code, I am unsure what the concurrency requirements are here.
--
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 pull request #3713: HDDS-7166. Memory leak in Recon when replacing DB for new checkpoint
Posted by GitBox <gi...@apache.org>.
kerneltime commented on PR #3713:
URL: https://github.com/apache/ozone/pull/3713#issuecomment-1226603629
> > > Should the logic to close the DB be moved inside `initializeNewRdbStore`?
> >
> >
> > Thanks for the review, @kerneltime. I guess the closure of the existing DB should be in `updateOmDB(File newDbLocation)` as it's meant to replace the DB object. But I'm ok either way, think it can be better if `initializeNewRdbStore` only creates and initializes `DBStore` and doesn't attempt to `set`.
>
> I agree. It would be good to place the new DB, initialize it, and then make it available. Looking at the code, I am unsure what the concurrency requirements are here.
Looks like It is single-threaded. For now, this is fine, so gave it a ship it.
--
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 merged pull request #3713: HDDS-7166. Memory leak in Recon when replacing DB for new checkpoint
Posted by GitBox <gi...@apache.org>.
kerneltime merged PR #3713:
URL: https://github.com/apache/ozone/pull/3713
--
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 pull request #3713: HDDS-7166. Memory leak in Recon when replacing DB for new checkpoint
Posted by GitBox <gi...@apache.org>.
kerneltime commented on PR #3713:
URL: https://github.com/apache/ozone/pull/3713#issuecomment-1226225548
Should the logic to close the DB be moved inside `initializeNewRdbStore`?
--
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] duongnguyen0 commented on pull request #3713: HDDS-7166. Memory leak in Recon when replacing DB for new checkpoint
Posted by GitBox <gi...@apache.org>.
duongnguyen0 commented on PR #3713:
URL: https://github.com/apache/ozone/pull/3713#issuecomment-1226431612
> Should the logic to close the DB be moved inside `initializeNewRdbStore`?
Thanks for the review, @kerneltime. I guess the closure of the existing DB should be in `updateOmDB(File newDbLocation)` as it's meant to replace the DB object. But I'm ok either way, think it can be better if `initializeNewRdbStore` only creates and initializes `DBStore` and doesn't attempt to `set`.
--
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