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 2020/08/14 16:10:17 UTC
[GitHub] [kafka] chia7712 opened a new pull request #9182: KAFKA-10403 Replace scala collection by java collection in generating…
chia7712 opened a new pull request #9182:
URL: https://github.com/apache/kafka/pull/9182
issue: https://issues.apache.org/jira/browse/KAFKA-10403
It seems to me the metrics is a "kind" of public interface so users should be able to access metrics of kafka server without scala library.
### 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on pull request #9182: KAFKA-10403 Replace scala collection by java collection in generating…
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #9182:
URL: https://github.com/apache/kafka/pull/9182#issuecomment-674414560
```javadoc: error - Error fetching URL: https://docs.oracle.com/en/java/javase/15/docs/api/```
The link of API is a bit weird to me. I have filed #9185 to fix 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on pull request #9182: KAFKA-10403 Replace scala collection by java collection in generating…
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #9182:
URL: https://github.com/apache/kafka/pull/9182#issuecomment-688132941
```
Build / JDK 8 / org.apache.kafka.connect.mirror.MirrorConnectorsIntegrationTest.testReplication
```
unrelated 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on pull request #9182: KAFKA-10403 Replace scala collection by java collection in generating…
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #9182:
URL: https://github.com/apache/kafka/pull/9182#issuecomment-689256679
```
Execution failed for task ':connect:mirror:integrationTest'.
01:19:16 > Process 'Gradle Test Executor 48' finished with non-zero exit value 1
01:19:16 This problem might be caused by incorrect test process configuration.
01:19:16 Please refer to the test execution section in the User Manual at
```
the error is unrelated to this PR
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on pull request #9182: KAFKA-10403 Replace scala collection by java collection in generating…
Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #9182:
URL: https://github.com/apache/kafka/pull/9182#issuecomment-688916053
The change looks good, can you please provide a test as well?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on pull request #9182: KAFKA-10403 Replace scala collection by java collection in generating…
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #9182:
URL: https://github.com/apache/kafka/pull/9182#issuecomment-679805021
retest this please
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on pull request #9182: KAFKA-10403 Replace scala collection by java collection in generating…
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #9182:
URL: https://github.com/apache/kafka/pull/9182#issuecomment-675344226
retest this please
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on a change in pull request #9182: KAFKA-10403 Replace scala collection by java collection in generating…
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #9182:
URL: https://github.com/apache/kafka/pull/9182#discussion_r484981150
##########
File path: core/src/main/scala/kafka/utils/Log4jController.scala
##########
@@ -87,9 +87,10 @@ object Log4jController {
class Log4jController extends Log4jControllerMBean {
def getLoggers: util.List[String] = {
- Log4jController.loggers.map {
+ // we replace scala collection by java collection so mbean client is able to parse it without scala library.
Review comment:
Instead of `parse`, we should say `deserialize`.
##########
File path: core/src/test/scala/kafka/utils/LoggingTest.scala
##########
@@ -26,6 +26,12 @@ import org.junit.Assert.{assertEquals, assertTrue}
class LoggingTest extends Logging {
+ @Test
+ def testTypeOfGetLoggers(): Unit = {
Review comment:
Can we add a comment to the test too since this is not obvious.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma merged pull request #9182: KAFKA-10403 Replace scala collection by java collection in generating…
Posted by GitBox <gi...@apache.org>.
ijuma merged pull request #9182:
URL: https://github.com/apache/kafka/pull/9182
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on pull request #9182: KAFKA-10403 Replace scala collection by java collection in generating…
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #9182:
URL: https://github.com/apache/kafka/pull/9182#issuecomment-674370125
> Can you please include more detail on how it fails currently? I assume it's some kind of deserialization error?
You are right. The root cause is deserialization error. Currently, the type of collection used by ```Log4jController#getLoggers ``` (a MBean object) is scala collection (```toList.asJava```) rather than java collection. Hence, the profiler has to load scala library (scala-library.jar) to deserialize the binary to origin scala collection. For example, if I don't add scala library to JMC's classpath, JMC fails to deserialize the attribute of ```Log4jController``` and it shows "N/A" instead.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on pull request #9182: KAFKA-10403 Replace scala collection by java collection in generating…
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #9182:
URL: https://github.com/apache/kafka/pull/9182#issuecomment-688927747
> can you please provide a test as well?
done!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on pull request #9182: KAFKA-10403 Replace scala collection by java collection in generating…
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #9182:
URL: https://github.com/apache/kafka/pull/9182#issuecomment-678589736
failed tests pass on my local
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] ijuma commented on pull request #9182: KAFKA-10403 Replace scala collection by java collection in generating…
Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #9182:
URL: https://github.com/apache/kafka/pull/9182#issuecomment-674160788
Can you please include more detail on how it fails currently? I assume it's some kind of deserialization 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on pull request #9182: KAFKA-10403 Replace scala collection by java collection in generating…
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #9182:
URL: https://github.com/apache/kafka/pull/9182#issuecomment-687725363
```
Build / JDK 11 / org.apache.kafka.streams.integration.KStreamAggregationIntegrationTest.shouldAggregateSlidingWindows
Build / JDK 11 / org.apache.kafka.streams.integration.KStreamAggregationIntegrationTest.shouldAggregateSlidingWindows
Build / JDK 15 / org.apache.kafka.streams.integration.KStreamAggregationIntegrationTest.shouldAggregateSlidingWindows
Build / JDK 15 / org.apache.kafka.streams.integration.KStreamAggregationIntegrationTest.shouldAggregateSlidingWindows
```
unrelated error
@ijuma Could you take a look 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org