Re: [PR] KAFKA-16002: Implement RemoteCopyLagSegments, RemoteDeleteLagBytes and RemoteDeleteLagSegments [kafka]

showuon merged PR #15005: URL: https://github.com/apache/kafka/pull/15005 -- 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.

Re: [PR] KAFKA-16002: Implement RemoteCopyLagSegments, RemoteDeleteLagBytes and RemoteDeleteLagSegments [kafka]

showuon commented on PR #15005: URL: https://github.com/apache/kafka/pull/15005#issuecomment-1865598500 Failed tests are unrelated. -- 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 co

Re: [PR] KAFKA-16002: Implement RemoteCopyLagSegments, RemoteDeleteLagBytes and RemoteDeleteLagSegments [kafka]

showuon commented on PR #15005: URL: https://github.com/apache/kafka/pull/15005#issuecomment-1864365020 I've confirmed `testRemoteLogManagerRemoteMetrics()` failure is not related to this change. I've reverted the test in https://github.com/apache/kafka/pull/15015 and created https://issue

Re: [PR] KAFKA-16002: Implement RemoteCopyLagSegments, RemoteDeleteLagBytes and RemoteDeleteLagSegments [kafka]

kamalcph commented on PR #15005: URL: https://github.com/apache/kafka/pull/15005#issuecomment-1863858511 https://github.com/apache/kafka/pull/15005#discussion_r1431092107 We can take this up in the next PR, otherwise the replica might report stale metrics. -- This is an automated

Re: [PR] KAFKA-16002: Implement RemoteCopyLagSegments, RemoteDeleteLagBytes and RemoteDeleteLagSegments [kafka]

clolov commented on code in PR #15005: URL: https://github.com/apache/kafka/pull/15005#discussion_r1431169575 ## core/src/main/java/kafka/log/remote/RemoteLogManager.java: ## @@ -1071,15 +1085,38 @@ void cleanupExpiredRemoteLogSegments() throws RemoteStorageException, Execution

Re: [PR] KAFKA-16002: Implement RemoteCopyLagSegments, RemoteDeleteLagBytes and RemoteDeleteLagSegments [kafka]

kamalcph commented on code in PR #15005: URL: https://github.com/apache/kafka/pull/15005#discussion_r1431092107 ## core/src/main/java/kafka/log/remote/RemoteLogManager.java: ## @@ -1071,15 +1085,38 @@ void cleanupExpiredRemoteLogSegments() throws RemoteStorageException, Executi

Re: [PR] KAFKA-16002: Implement RemoteCopyLagSegments, RemoteDeleteLagBytes and RemoteDeleteLagSegments [kafka]

clolov commented on code in PR #15005: URL: https://github.com/apache/kafka/pull/15005#discussion_r1431012760 ## core/src/main/java/kafka/log/remote/RemoteLogManager.java: ## @@ -1071,15 +1085,38 @@ void cleanupExpiredRemoteLogSegments() throws RemoteStorageException, Execution

Re: [PR] KAFKA-16002: Implement RemoteCopyLagSegments, RemoteDeleteLagBytes and RemoteDeleteLagSegments [kafka]

kamalcph commented on code in PR #15005: URL: https://github.com/apache/kafka/pull/15005#discussion_r1430784781 ## core/src/main/java/kafka/log/remote/RemoteLogManager.java: ## @@ -1071,15 +1085,38 @@ void cleanupExpiredRemoteLogSegments() throws RemoteStorageException, Executi

Re: [PR] KAFKA-16002: Implement RemoteCopyLagSegments, RemoteDeleteLagBytes and RemoteDeleteLagSegments [kafka]

clolov commented on code in PR #15005: URL: https://github.com/apache/kafka/pull/15005#discussion_r1430372576 ## core/src/main/java/kafka/log/remote/RemoteLogManager.java: ## @@ -1073,13 +1088,28 @@ void cleanupExpiredRemoteLogSegments() throws RemoteStorageException, Execution

Re: [PR] KAFKA-16002: Implement RemoteCopyLagSegments, RemoteDeleteLagBytes and RemoteDeleteLagSegments [kafka]

clolov commented on code in PR #15005: URL: https://github.com/apache/kafka/pull/15005#discussion_r1430372576 ## core/src/main/java/kafka/log/remote/RemoteLogManager.java: ## @@ -1073,13 +1088,28 @@ void cleanupExpiredRemoteLogSegments() throws RemoteStorageException, Execution

Re: [PR] KAFKA-16002: Implement RemoteCopyLagSegments, RemoteDeleteLagBytes and RemoteDeleteLagSegments [kafka]

showuon commented on PR #15005: URL: https://github.com/apache/kafka/pull/15005#issuecomment-1860035026 @clolov , could you address @kamalcph 's comments above? Thanks. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use t

Re: [PR] KAFKA-16002: Implement RemoteCopyLagSegments, RemoteDeleteLagBytes and RemoteDeleteLagSegments [kafka]

kamalcph commented on PR #15005: URL: https://github.com/apache/kafka/pull/15005#issuecomment-1858783152 > This is a good point, but I think adding a lock is overkilling compared with the value discrepancy. I think the value deviation should be acceptable compared with sacrificing the perfo

Re: [PR] KAFKA-16002: Implement RemoteCopyLagSegments, RemoteDeleteLagBytes and RemoteDeleteLagSegments [kafka]

kamalcph commented on code in PR #15005: URL: https://github.com/apache/kafka/pull/15005#discussion_r1428765635 ## core/src/main/java/kafka/log/remote/RemoteLogManager.java: ## @@ -1073,13 +1088,28 @@ void cleanupExpiredRemoteLogSegments() throws RemoteStorageException, Executi

Re: [PR] KAFKA-16002: Implement RemoteCopyLagSegments, RemoteDeleteLagBytes and RemoteDeleteLagSegments [kafka]

kamalcph commented on code in PR #15005: URL: https://github.com/apache/kafka/pull/15005#discussion_r1428764470 ## core/src/main/java/kafka/log/remote/RemoteLogManager.java: ## @@ -1073,13 +1087,29 @@ void cleanupExpiredRemoteLogSegments() throws RemoteStorageException, Executi

Re: [PR] KAFKA-16002: Implement RemoteCopyLagSegments, RemoteDeleteLagBytes and RemoteDeleteLagSegments [kafka]

clolov commented on PR #15005: URL: https://github.com/apache/kafka/pull/15005#issuecomment-1858766576 Okay, the latest commits should have no checkstyle or spotbugs problems + are on top of the latest trunk! -- This is an automated message from the Apache Git Service. To respond to the m

Re: [PR] KAFKA-16002: Implement RemoteCopyLagSegments, RemoteDeleteLagBytes and RemoteDeleteLagSegments [kafka]

clolov commented on code in PR #15005: URL: https://github.com/apache/kafka/pull/15005#discussion_r1428742212 ## core/src/main/java/kafka/log/remote/RemoteLogManager.java: ## @@ -1073,13 +1088,28 @@ void cleanupExpiredRemoteLogSegments() throws RemoteStorageException, Execution

Re: [PR] KAFKA-16002: Implement RemoteCopyLagSegments, RemoteDeleteLagBytes and RemoteDeleteLagSegments [kafka]

clolov commented on PR #15005: URL: https://github.com/apache/kafka/pull/15005#issuecomment-1858760305 Yeah, I am with Luke on this one. I believe that taking a lock to report metrics is almost always unnecessary - i.e. I treat metrics as eventually consistent. I will go over the rest of th

Re: [PR] KAFKA-16002: Implement RemoteCopyLagSegments, RemoteDeleteLagBytes and RemoteDeleteLagSegments [kafka]

showuon commented on PR #15005: URL: https://github.com/apache/kafka/pull/15005#issuecomment-1858738944 > The following metrics are related to each other: > > 1. RemoteCopyLagBytes and RemoteCopyLagSegments > > 2. RemoteDeleteLagBytes and RemoteDeleteLagSegments >

Re: [PR] KAFKA-16002: Implement RemoteCopyLagSegments, RemoteDeleteLagBytes and RemoteDeleteLagSegments [kafka]

kamalcph commented on code in PR #15005: URL: https://github.com/apache/kafka/pull/15005#discussion_r1428638499 ## core/src/main/java/kafka/log/remote/RemoteLogManager.java: ## @@ -1073,13 +1088,28 @@ void cleanupExpiredRemoteLogSegments() throws RemoteStorageException, Executi

Re: [PR] KAFKA-16002: Implement RemoteCopyLagSegments, RemoteDeleteLagBytes and RemoteDeleteLagSegments [kafka]

kamalcph commented on PR #15005: URL: https://github.com/apache/kafka/pull/15005#issuecomment-1858669208 The following metrics are related to each other: 1. RemoteCopyBytesLag and RemoteCopySegmentsLag 2. RemoteDeleteBytesLag and RemoteDeleteSegmentsLag We should update their

Re: [PR] KAFKA-16002: Implement RemoteCopyLagSegments, RemoteDeleteLagBytes and RemoteDeleteLagSegments [kafka]

kamalcph commented on PR #15005: URL: https://github.com/apache/kafka/pull/15005#issuecomment-1857818458 I'll provide a review today. -- 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

Re: [PR] KAFKA-16002: Implement RemoteCopyLagSegments, RemoteDeleteLagBytes and RemoteDeleteLagSegments [kafka]

showuon commented on PR #15005: URL: https://github.com/apache/kafka/pull/15005#issuecomment-1857734261 @clolov , there are checkstyle error. Please fix them: https://ci-builds.apache.org/blue/organizations/jenkins/Kafka%2Fkafka-pr/detail/PR-15005/7/pipeline . Thanks. -- This is an autom

Re: [PR] KAFKA-16002: Implement RemoteCopyLagSegments, RemoteDeleteLagBytes and RemoteDeleteLagSegments [kafka]

clolov commented on code in PR #15005: URL: https://github.com/apache/kafka/pull/15005#discussion_r1427814012 ## core/src/main/java/kafka/log/remote/RemoteLogManager.java: ## @@ -1073,13 +1088,29 @@ void cleanupExpiredRemoteLogSegments() throws RemoteStorageException, Execution

Re: [PR] KAFKA-16002: Implement RemoteCopyLagSegments, RemoteDeleteLagBytes and RemoteDeleteLagSegments [kafka]

showuon commented on code in PR #15005: URL: https://github.com/apache/kafka/pull/15005#discussion_r1427637469 ## core/src/main/java/kafka/log/remote/RemoteLogManager.java: ## @@ -1073,13 +1088,29 @@ void cleanupExpiredRemoteLogSegments() throws RemoteStorageException, Executio

Re: [PR] KAFKA-16002: Implement RemoteCopyLagSegments, RemoteDeleteLagBytes and RemoteDeleteLagSegments [kafka]

clolov commented on PR #15005: URL: https://github.com/apache/kafka/pull/15005#issuecomment-1856057285 I believe the pull request is now ready for an official review @showuon 😊 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub

Re: [PR] KAFKA-16002: Implement RemoteCopyLagSegments, RemoteDeleteLagBytes and RemoteDeleteLagSegments [kafka]

clolov commented on code in PR #15005: URL: https://github.com/apache/kafka/pull/15005#discussion_r1426785644 ## core/src/main/java/kafka/log/remote/RemoteLogManager.java: ## @@ -1041,10 +1060,20 @@ void cleanupExpiredRemoteLogSegments() throws RemoteStorageException, Execution

Re: [PR] KAFKA-16002: Implement RemoteCopyLagSegments, RemoteDeleteLagBytes and RemoteDeleteLagSegments [kafka]

showuon commented on code in PR #15005: URL: https://github.com/apache/kafka/pull/15005#discussion_r1426233918 ## core/src/main/java/kafka/log/remote/RemoteLogManager.java: ## @@ -1041,10 +1060,20 @@ void cleanupExpiredRemoteLogSegments() throws RemoteStorageException, Executio

Re: [PR] KAFKA-16002: Implement RemoteCopyLagSegments, RemoteDeleteLagBytes and RemoteDeleteLagSegments [kafka]

clolov commented on PR #15005: URL: https://github.com/apache/kafka/pull/15005#issuecomment-1854551258 Heya @showuon and @kamalcph! I have marked this as a draft, but will aim to add the needed test + functionality tomorrow. I would appreciate any initial reviews and suggestions for improve