Re: [PR] KAFKA-16696: Removed the in-memory implementation of RSM and RLMM [kafka]
chia7712 merged PR #15911: URL: https://github.com/apache/kafka/pull/15911 -- 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
Re: [PR] KAFKA-16696: Removed the in-memory implementation of RSM and RLMM [kafka]
chia7712 commented on PR #15911: URL: https://github.com/apache/kafka/pull/15911#issuecomment-2107235668 > Okay, let's go forward with this so that you are not blocked on me, I will try to figure out a way forward and if not I will write again :) @clolov thanks! Please feel free to ping me if you have a draft PR. I'd like to make `ClusterInstance` works for more test cases. -- 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
Re: [PR] KAFKA-16696: Removed the in-memory implementation of RSM and RLMM [kafka]
clolov commented on PR #15911: URL: https://github.com/apache/kafka/pull/15911#issuecomment-2107010294 Okay, let's go forward with this so that you are not blocked on me, I will try to figure out a way forward and if not I will write again :) -- 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
Re: [PR] KAFKA-16696: Removed the in-memory implementation of RSM and RLMM [kafka]
chia7712 commented on PR #15911: URL: https://github.com/apache/kafka/pull/15911#issuecomment-2104790646 > Can you actually not remove this because I was starting to use it in the GetOffsetShellToolTest? The problem I faced with using the topic-based RLMM is that it requires the bootstrap-servers to initialise correctly. However, said bootstrap-servers are not present at initialisation time (due to determining the port dynamically) @clolov Could you take a look at #15917 ? I try to use another way to write tests for storage module. `ClusterInstace` get created and port is bound in testing phase. Maybe that can fix problem you described. -- 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
Re: [PR] KAFKA-16696: Removed the in-memory implementation of RSM and RLMM [kafka]
kamalcph commented on PR #15911: URL: https://github.com/apache/kafka/pull/15911#issuecomment-2104768094 > Can you actually not remove this because I was starting to use it in the GetOffsetShellToolTest? The problem I faced with using the topic-based RLMM is that it requires the bootstrap-servers to initialise correctly. However, said bootstrap-servers are not present at initialisation time (due to determining the port dynamically) We can get the port from the Kafkabroker using the `boundPort` method. If it does not work for you, Could you please open a draft PR? I'll take a look. -- 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
Re: [PR] KAFKA-16696: Removed the in-memory implementation of RSM and RLMM [kafka]
clolov commented on PR #15911: URL: https://github.com/apache/kafka/pull/15911#issuecomment-2104745407 Can you actually not remove this because I was starting to use it in the GetOffsetShellTool? The problem I faced with using the topic-based RLMM is that it requires the bootstrap-servers to start correctly. However, said bootstrap-servers are not present at initialisation time (due to determining the port dynamically) -- 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
Re: [PR] KAFKA-16696: Removed the in-memory implementation of RSM and RLMM [kafka]
chia7712 commented on code in PR #15911:
URL: https://github.com/apache/kafka/pull/15911#discussion_r1596516913
##
storage/src/test/java/org/apache/kafka/server/log/remote/storage/RemoteLogMetadataManagerTest.java:
##
@@ -49,9 +45,10 @@ public class RemoteLogMetadataManagerTest {
private final Time time = new MockTime(1);
-@ParameterizedTest(name = "remoteLogMetadataManager = {0}")
-@MethodSource("remoteLogMetadataManagers")
-public void testFetchSegments(RemoteLogMetadataManager
remoteLogMetadataManager) throws Exception {
+private RemoteLogMetadataManager remoteLogMetadataManager = new
TopicBasedRemoteLogMetadataManagerWrapperWithHarness();
+
+@Test
+public void testFetchSegments() throws Exception {
try {
Review Comment:
This can be addressed as follow-up I think :)
--
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
Re: [PR] KAFKA-16696: Removed the in-memory implementation of RSM and RLMM [kafka]
chia7712 commented on PR #15911: URL: https://github.com/apache/kafka/pull/15911#issuecomment-2104271236 QA is re-triggered, and I will merge it if QA does not show the related error. -- 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
Re: [PR] KAFKA-16696: Removed the in-memory implementation of RSM and RLMM [kafka]
chia7712 commented on code in PR #15911:
URL: https://github.com/apache/kafka/pull/15911#discussion_r1596287627
##
storage/src/test/java/org/apache/kafka/server/log/remote/storage/RemoteLogMetadataManagerTest.java:
##
@@ -49,9 +45,10 @@ public class RemoteLogMetadataManagerTest {
private final Time time = new MockTime(1);
-@ParameterizedTest(name = "remoteLogMetadataManager = {0}")
-@MethodSource("remoteLogMetadataManagers")
-public void testFetchSegments(RemoteLogMetadataManager
remoteLogMetadataManager) throws Exception {
+private RemoteLogMetadataManager remoteLogMetadataManager = new
TopicBasedRemoteLogMetadataManagerWrapperWithHarness();
+
+@Test
+public void testFetchSegments() throws Exception {
try {
Review Comment:
It seems to me `TopicBasedRemoteLogMetadataManagerWrapperWithHarness` can be
removed also. We don't use the wrapper actually. This test can be modified by
following style:
```java
@Test
public void testFetchSegments() throws Exception {
try (TopicBasedRemoteLogMetadataManagerHarness
remoteLogMetadataManagerHarness = new
TopicBasedRemoteLogMetadataManagerHarness()) {
RemoteLogMetadataManager remoteLogMetadataManager =
remoteLogMetadataManagerHarness.remoteLogMetadataManager();
```
noted `TopicBasedRemoteLogMetadataManagerHarness` needs to implement
`AutoClosable`, and `remoteLogMetadataManager` should be a public method.
--
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
[PR] KAFKA-16696: Removed the in-memory implementation of RSM and RLMM [kafka]
kamalcph opened a new pull request, #15911: URL: https://github.com/apache/kafka/pull/15911 ### Committer Checklist (excluded from commit message) - [ ] Verify design and implementation - [ ] Verify test coverage and CI build status - [ ] Verify documentation (including upgrade notes) -- 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
