[GitHub] [kafka] Owen-CH-Leung commented on pull request #14136: Add metadatacache into RemoteLogManager, and refactor all relevant codes
Owen-CH-Leung commented on PR #14136: URL: https://github.com/apache/kafka/pull/14136#issuecomment-1685205287 @clolov @kamalcph I've merged base and refactored the test suite a bit. There're a few failed test but I believe those are unrelated to the changes I made. Could you please take a look and let me know your comments ? -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [kafka] Owen-CH-Leung commented on pull request #14136: Add metadatacache into RemoteLogManager, and refactor all relevant codes
Owen-CH-Leung commented on PR #14136: URL: https://github.com/apache/kafka/pull/14136#issuecomment-1681516264 > Overall LGTM. Can we hold this patch until #13947 lands? Otherwise, this patch needs one integration test to verify end-to-end change. No problem. I can perform rebase after the PR was merged -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [kafka] Owen-CH-Leung commented on pull request #14136: Add metadatacache into RemoteLogManager, and refactor all relevant codes
Owen-CH-Leung commented on PR #14136: URL: https://github.com/apache/kafka/pull/14136#issuecomment-1663794337 Oh yesThanks and sorry for the dumb question! I've fixed the CI failing test and all should be good for review now. Let me know your feedback. -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [kafka] Owen-CH-Leung commented on pull request #14136: Add metadatacache into RemoteLogManager, and refactor all relevant codes
Owen-CH-Leung commented on PR #14136: URL: https://github.com/apache/kafka/pull/14136#issuecomment-1661525755 Hello @divijvaidya , I'm still in the process of writing the PR and would like to ask something : For this ticket, in order to reuse the metadata cache inside broker (which I believe is already instantiated when the broker server is set up), I think I need to access the following factory method code snippet : https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/server/MetadataCache.scala#L114-L125 However, this method will always return a new instance of metadatacache instead of the one being instantiated inside broker. So does it make sense to change it to a Singleton class ? -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
