You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2022/11/08 22:07:02 UTC
[GitHub] [kafka] cmccabe opened a new pull request, #12833: KAFKA-14370: Properly close ImageWriter objects
cmccabe opened a new pull request, #12833:
URL: https://github.com/apache/kafka/pull/12833
Properly close ImageWriter objects in BrokerMetadataListener and BrokerMetadataSnapshotter. Add a time limit on BrokerMetadataSnapshotterTest.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] cmccabe commented on a diff in pull request #12833: KAFKA-14370: Properly close ImageWriter objects
Posted by GitBox <gi...@apache.org>.
cmccabe commented on code in PR #12833:
URL: https://github.com/apache/kafka/pull/12833#discussion_r1018269186
##########
core/src/main/scala/kafka/server/metadata/BrokerMetadataListener.scala:
##########
@@ -398,8 +398,11 @@ class BrokerMetadataListener(
build()
try {
_image.write(writer, options)
- } finally {
- writer.close()
+ writer.close(true)
Review Comment:
Yes, you are right that we're already calling `ImageWriter.close(true)` in `MetadataImage.write`
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] jsancio commented on a diff in pull request #12833: KAFKA-14370: Properly close ImageWriter objects
Posted by GitBox <gi...@apache.org>.
jsancio commented on code in PR #12833:
URL: https://github.com/apache/kafka/pull/12833#discussion_r1017191691
##########
core/src/main/scala/kafka/server/metadata/BrokerMetadataListener.scala:
##########
@@ -398,8 +398,11 @@ class BrokerMetadataListener(
build()
try {
_image.write(writer, options)
- } finally {
- writer.close()
+ writer.close(true)
Review Comment:
`MetadataImage::write` calls `ImageWriter::close(true)`. Can we remove that call? It if very difficult to read and understand code that closes `AutoCloseable` in different layers of the code.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] github-actions[bot] commented on pull request #12833: KAFKA-14370: Simplify the ImageWriter API by adding freeze
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12833:
URL: https://github.com/apache/kafka/pull/12833#issuecomment-1616333071
This PR is being marked as stale since it has not had any activity in 90 days. If you would like to keep this PR alive, please ask a committer for review. If the PR has merge conflicts, please update it with the latest from trunk (or appropriate release branch) <p> If this PR is no longer valid or desired, please feel free to close it. If no activity occurrs in the next 30 days, it will be automatically closed.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] cmccabe commented on pull request #12833: KAFKA-14370: Simplify the ImageWriter API by adding freeze
Posted by GitBox <gi...@apache.org>.
cmccabe commented on PR #12833:
URL: https://github.com/apache/kafka/pull/12833#issuecomment-1309197714
It's a bit confusing to have two variations of close(), so let's just have freeze() and then close().
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-14370: Simplify the ImageWriter API by adding freeze [kafka]
Posted by "cmccabe (via GitHub)" <gi...@apache.org>.
cmccabe closed pull request #12833: KAFKA-14370: Simplify the ImageWriter API by adding freeze
URL: https://github.com/apache/kafka/pull/12833
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org