You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@pekko.apache.org by "He-Pin (via GitHub)" <gi...@apache.org> on 2023/08/05 09:21:11 UTC
[GitHub] [incubator-pekko] He-Pin opened a new pull request, #539: !test Migrate multi node testkit to Netty 4.
He-Pin opened a new pull request, #539:
URL: https://github.com/apache/incubator-pekko/pull/539
refs: https://github.com/apache/incubator-pekko/issues/462
I verified it locally withcluster/MultiJvm/test
previous: https://github.com/apache/incubator-pekko/pull/486
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] He-Pin commented on a diff in pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#discussion_r1285186265
##########
project/SbtMultiJvm.scala:
##########
@@ -372,11 +374,41 @@ object MultiJvmPlugin extends AutoPlugin {
val connectInput = input && index == 0
log.debug("Starting %s for %s".format(jvmName, testClass))
log.debug(" with JVM options: %s".format(allJvmOptions.mkString(" ")))
- (testClass, Jvm.startJvm(javaBin, allJvmOptions, runOptions, jvmLogger, connectInput))
+ val testClass2Process = (testClass, Jvm.startJvm(javaBin, allJvmOptions, runOptions, jvmLogger, connectInput))
+ if (index == 0) {
+ log.debug("%s for %s 's started as `Controller`, waiting before can be connected for clients.".format(jvmName,
+ testClass))
+ val controllerHost = hosts.head
+ val serverPort: Int = Integer.getInteger("multinode.server-port", 4711)
+ waitingBeforeConnectable(controllerHost, serverPort, TimeUnit.SECONDS.toMillis(20L))
+ }
+ testClass2Process
}
processExitCodes(name, processes, log)
}
+ private def waitingBeforeConnectable(host: String, port: Int, timeoutInMillis: Long): Unit = {
+ val inetSocketAddress = new InetSocketAddress(host, port)
+ def telnet(addr: InetSocketAddress, timeout: Int): Boolean = {
+ val socket: Socket = new Socket()
+ try {
+ socket.connect(inetSocketAddress, timeout)
+ socket.isConnected
+ } catch {
+ case _: Exception => false
+ } finally {
+ socket.close()
+ }
+ }
+
+ val startTime = System.currentTimeMillis()
+ var connectivity = false
+ while (!connectivity && (System.currentTimeMillis() - startTime < timeoutInMillis)) {
Review Comment:
I think that's ok, as it's just in tests and no one will touch those again, otherwise we need another：
```
multinode.max-time-waiting-to-connect-in-seconds=20
```
like thing
##########
project/SbtMultiJvm.scala:
##########
@@ -372,11 +374,41 @@ object MultiJvmPlugin extends AutoPlugin {
val connectInput = input && index == 0
log.debug("Starting %s for %s".format(jvmName, testClass))
log.debug(" with JVM options: %s".format(allJvmOptions.mkString(" ")))
- (testClass, Jvm.startJvm(javaBin, allJvmOptions, runOptions, jvmLogger, connectInput))
+ val testClass2Process = (testClass, Jvm.startJvm(javaBin, allJvmOptions, runOptions, jvmLogger, connectInput))
+ if (index == 0) {
+ log.debug("%s for %s 's started as `Controller`, waiting before can be connected for clients.".format(jvmName,
+ testClass))
+ val controllerHost = hosts.head
+ val serverPort: Int = Integer.getInteger("multinode.server-port", 4711)
+ waitingBeforeConnectable(controllerHost, serverPort, TimeUnit.SECONDS.toMillis(20L))
+ }
+ testClass2Process
}
processExitCodes(name, processes, log)
}
+ private def waitingBeforeConnectable(host: String, port: Int, timeoutInMillis: Long): Unit = {
+ val inetSocketAddress = new InetSocketAddress(host, port)
+ def telnet(addr: InetSocketAddress, timeout: Int): Boolean = {
+ val socket: Socket = new Socket()
+ try {
+ socket.connect(inetSocketAddress, timeout)
+ socket.isConnected
+ } catch {
+ case _: Exception => false
+ } finally {
+ socket.close()
+ }
+ }
+
+ val startTime = System.currentTimeMillis()
+ var connectivity = false
+ while (!connectivity && (System.currentTimeMillis() - startTime < timeoutInMillis)) {
Review Comment:
I think that's ok, as it's just in tests and no one will touch those again, otherwise we need another：
```
multinode.max-time-waiting-to-connect-in-seconds=20
```
like thing
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] He-Pin commented on pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#issuecomment-1666960184
I ran it several times again and it do passed, so it would be good to go.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] He-Pin commented on pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#issuecomment-1666927569
I run several times on gitpod. it works, I will do a final validation before merge this.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] mdedetrich commented on a diff in pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#discussion_r1285189073
##########
project/SbtMultiJvm.scala:
##########
@@ -167,10 +168,11 @@ object MultiJvmPlugin extends AutoPlugin {
// the first class wins just like a classpath
// just concatenate conflicting text files
assembly / assemblyMergeStrategy := {
- case n if n.endsWith(".class") => MergeStrategy.first
- case n if n.endsWith(".txt") => MergeStrategy.concat
- case n if n.endsWith("NOTICE") => MergeStrategy.concat
- case n => (assembly / assemblyMergeStrategy).value.apply(n)
+ case n if n.endsWith(".class") => MergeStrategy.first
+ case n if n.endsWith(".txt") => MergeStrategy.concat
+ case n if n.endsWith("NOTICE") => MergeStrategy.concat
+ case n if n.endsWith("LICENSE") => MergeStrategy.concat
Review Comment:
Yes please make a separate PR for this. Although it's true that this only effects netty 4 for now, in general if there multiple `LICENSE`'s we do want to concat them (i.e. this isn't just specific to netty 4)
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] He-Pin merged pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin merged PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] pjfanning commented on a diff in pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#discussion_r1285186114
##########
multi-node-testkit/src/main/scala/org/apache/pekko/remote/testconductor/RemoteConnection.scala:
##########
@@ -94,44 +95,88 @@ private[pekko] case object Client extends Role
*/
private[pekko] case object Server extends Role
+/**
+ * INTERNAL API.
+ */
+private[pekko] trait RemoteConnection {
+
+ /**
+ * The channel future associated with this connection.
+ */
+ def channelFuture: ChannelFuture
+
+ /**
+ * Shutdown the connection and release the resources.
+ */
+ def shutdown(): Unit
+}
+
/**
* INTERNAL API.
*/
private[pekko] object RemoteConnection {
- def apply(role: Role, sockaddr: InetSocketAddress, poolSize: Int, handler: ChannelUpstreamHandler): Channel = {
+ def apply(
+ role: Role,
+ sockaddr: InetSocketAddress,
+ poolSize: Int,
+ handler: ChannelInboundHandler): RemoteConnection = {
role match {
case Client =>
- val socketfactory =
- new NioClientSocketChannelFactory(Executors.newCachedThreadPool, Executors.newCachedThreadPool, poolSize)
- val bootstrap = new ClientBootstrap(socketfactory)
- bootstrap.setPipelineFactory(new TestConductorPipelineFactory(handler))
- bootstrap.setOption("tcpNoDelay", true)
- bootstrap.connect(sockaddr).getChannel
+ val bootstrap = new Bootstrap()
+ val eventLoopGroup = new NioEventLoopGroup(poolSize)
+ val cf = bootstrap
+ .group(eventLoopGroup)
+ .channel(classOf[NioSocketChannel])
+ .handler(new TestConductorPipelineFactory(handler))
+ .option[java.lang.Boolean](ChannelOption.TCP_NODELAY, true)
+ .option[java.lang.Boolean](ChannelOption.SO_KEEPALIVE, true)
+ .connect(sockaddr)
+ new RemoteConnection {
+ override def channelFuture: ChannelFuture = cf
+
+ override def shutdown(): Unit = {
+ try {
+ channelFuture.channel().close().sync()
+ eventLoopGroup.shutdownGracefully(1L, 1L, TimeUnit.SECONDS)
+ } catch {
+ case NonFatal(_) => // silence this one to not make tests look like they failed, it's not really critical
+ }
+ }
+ }
+
case Server =>
- val socketfactory =
- new NioServerSocketChannelFactory(Executors.newCachedThreadPool, Executors.newCachedThreadPool, poolSize)
- val bootstrap = new ServerBootstrap(socketfactory)
- bootstrap.setPipelineFactory(new TestConductorPipelineFactory(handler))
- bootstrap.setOption("reuseAddress", !Helpers.isWindows)
- bootstrap.setOption("child.tcpNoDelay", true)
- bootstrap.bind(sockaddr)
+ val bootstrap = new ServerBootstrap()
+ val parentEventLoopGroup = new NioEventLoopGroup(poolSize)
+ val childEventLoopGroup = new NioEventLoopGroup(poolSize)
+ val cf = bootstrap
+ .group(parentEventLoopGroup, childEventLoopGroup)
+ .channel(classOf[NioServerSocketChannel])
+ .childHandler(new TestConductorPipelineFactory(handler))
+ .option[java.lang.Boolean](ChannelOption.SO_REUSEADDR, !Helpers.isWindows)
+ .option[java.lang.Integer](ChannelOption.SO_BACKLOG, 2048)
+ .childOption[java.lang.Boolean](ChannelOption.TCP_NODELAY, true)
+ .childOption[java.lang.Boolean](ChannelOption.SO_KEEPALIVE, true)
+ .bind(sockaddr)
+ .sync()
+
+ new RemoteConnection {
+ override def channelFuture: ChannelFuture = cf
+
+ override def shutdown(): Unit = {
+ try {
+ channelFuture.channel().close().sync()
+ parentEventLoopGroup.shutdownGracefully(1L, 1L, TimeUnit.SECONDS)
Review Comment:
I agree with @mdedetrich - this would be better as a setting in reference.conf
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] pjfanning commented on a diff in pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#discussion_r1285036594
##########
project/SbtMultiJvm.scala:
##########
@@ -372,7 +373,13 @@ object MultiJvmPlugin extends AutoPlugin {
val connectInput = input && index == 0
log.debug("Starting %s for %s".format(jvmName, testClass))
log.debug(" with JVM options: %s".format(allJvmOptions.mkString(" ")))
- (testClass, Jvm.startJvm(javaBin, allJvmOptions, runOptions, jvmLogger, connectInput))
+ val testClass2Process = (testClass, Jvm.startJvm(javaBin, allJvmOptions, runOptions, jvmLogger, connectInput))
+ if (index == 0) {
+ log.debug("%s for %s 's started as `Controller`, waiting 2 seconds before start clients.".format(jvmName,
+ testClass))
+ TimeUnit.SECONDS.sleep(2)
Review Comment:
I don't like time based waits. On a slow machine, they can stop working. Our CI builds on VMs that can be very slow.
Is there any way that this code could be changed to ping the netty cluster and keep doing so until it gets notification that the cluster is ready?
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] mdedetrich commented on a diff in pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#discussion_r1285175868
##########
cluster/src/multi-jvm/resources/logback-test.xml:
##########
@@ -0,0 +1,13 @@
+<configuration>
+ <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
+ <encoder>
+ <pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern>
+ </encoder>
+ </appender>
+
+ <root level="info">
+ <appender-ref ref="STDOUT" />
+ </root>
+ <logger name="io.netty.util.Recycler" level="ERROR" />
Review Comment:
Is this something that was needed just because of netty4 or is it generally helpful? If it is generally helpful I would do this change in a separate PR so that we can backport it to Pekko 1.0.x
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] He-Pin commented on a diff in pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#discussion_r1285184635
##########
cluster/src/multi-jvm/resources/logback-test.xml:
##########
@@ -0,0 +1,13 @@
+<configuration>
+ <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
+ <encoder>
+ <pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern>
+ </encoder>
+ </appender>
+
+ <root level="info">
+ <appender-ref ref="STDOUT" />
+ </root>
+ <logger name="io.netty.util.Recycler" level="ERROR" />
Review Comment:
It because there are so much netty log when you run testing.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] mdedetrich commented on a diff in pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#discussion_r1285185556
##########
cluster/src/multi-jvm/resources/logback-test.xml:
##########
@@ -0,0 +1,13 @@
+<configuration>
+ <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
+ <encoder>
+ <pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern>
+ </encoder>
+ </appender>
+
+ <root level="info">
+ <appender-ref ref="STDOUT" />
+ </root>
+ <logger name="io.netty.util.Recycler" level="ERROR" />
Review Comment:
There was a miscommunication, I realize that `PoolThreadCache` and `Recycler` classes only exist in Netty 4 and not Netty 3 (just checked now), resolving conversation.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] He-Pin commented on a diff in pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#discussion_r1285191268
##########
multi-node-testkit/src/main/scala/org/apache/pekko/remote/testconductor/RemoteConnection.scala:
##########
@@ -94,44 +95,88 @@ private[pekko] case object Client extends Role
*/
private[pekko] case object Server extends Role
+/**
+ * INTERNAL API.
+ */
+private[pekko] trait RemoteConnection {
+
+ /**
+ * The channel future associated with this connection.
+ */
+ def channelFuture: ChannelFuture
+
+ /**
+ * Shutdown the connection and release the resources.
+ */
+ def shutdown(): Unit
+}
+
/**
* INTERNAL API.
*/
private[pekko] object RemoteConnection {
- def apply(role: Role, sockaddr: InetSocketAddress, poolSize: Int, handler: ChannelUpstreamHandler): Channel = {
+ def apply(
+ role: Role,
+ sockaddr: InetSocketAddress,
+ poolSize: Int,
+ handler: ChannelInboundHandler): RemoteConnection = {
role match {
case Client =>
- val socketfactory =
- new NioClientSocketChannelFactory(Executors.newCachedThreadPool, Executors.newCachedThreadPool, poolSize)
- val bootstrap = new ClientBootstrap(socketfactory)
- bootstrap.setPipelineFactory(new TestConductorPipelineFactory(handler))
- bootstrap.setOption("tcpNoDelay", true)
- bootstrap.connect(sockaddr).getChannel
+ val bootstrap = new Bootstrap()
+ val eventLoopGroup = new NioEventLoopGroup(poolSize)
+ val cf = bootstrap
+ .group(eventLoopGroup)
+ .channel(classOf[NioSocketChannel])
+ .handler(new TestConductorPipelineFactory(handler))
+ .option[java.lang.Boolean](ChannelOption.TCP_NODELAY, true)
+ .option[java.lang.Boolean](ChannelOption.SO_KEEPALIVE, true)
+ .connect(sockaddr)
+ new RemoteConnection {
+ override def channelFuture: ChannelFuture = cf
+
+ override def shutdown(): Unit = {
+ try {
+ channelFuture.channel().close().sync()
+ eventLoopGroup.shutdownGracefully(1L, 1L, TimeUnit.SECONDS)
+ } catch {
+ case NonFatal(_) => // silence this one to not make tests look like they failed, it's not really critical
+ }
+ }
+ }
+
case Server =>
- val socketfactory =
- new NioServerSocketChannelFactory(Executors.newCachedThreadPool, Executors.newCachedThreadPool, poolSize)
- val bootstrap = new ServerBootstrap(socketfactory)
- bootstrap.setPipelineFactory(new TestConductorPipelineFactory(handler))
- bootstrap.setOption("reuseAddress", !Helpers.isWindows)
- bootstrap.setOption("child.tcpNoDelay", true)
- bootstrap.bind(sockaddr)
+ val bootstrap = new ServerBootstrap()
+ val parentEventLoopGroup = new NioEventLoopGroup(poolSize)
+ val childEventLoopGroup = new NioEventLoopGroup(poolSize)
+ val cf = bootstrap
+ .group(parentEventLoopGroup, childEventLoopGroup)
+ .channel(classOf[NioServerSocketChannel])
+ .childHandler(new TestConductorPipelineFactory(handler))
+ .option[java.lang.Boolean](ChannelOption.SO_REUSEADDR, !Helpers.isWindows)
+ .option[java.lang.Integer](ChannelOption.SO_BACKLOG, 2048)
+ .childOption[java.lang.Boolean](ChannelOption.TCP_NODELAY, true)
+ .childOption[java.lang.Boolean](ChannelOption.SO_KEEPALIVE, true)
+ .bind(sockaddr)
+ .sync()
+
+ new RemoteConnection {
+ override def channelFuture: ChannelFuture = cf
+
+ override def shutdown(): Unit = {
+ try {
+ channelFuture.channel().close().sync()
+ parentEventLoopGroup.shutdownGracefully(1L, 1L, TimeUnit.SECONDS)
Review Comment:
@mdedetrich I updated it to `.shutdownGracefully(0L, 0L, TimeUnit.SECONDS)` which alian the old behavior.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] mdedetrich commented on a diff in pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#discussion_r1285189073
##########
project/SbtMultiJvm.scala:
##########
@@ -167,10 +168,11 @@ object MultiJvmPlugin extends AutoPlugin {
// the first class wins just like a classpath
// just concatenate conflicting text files
assembly / assemblyMergeStrategy := {
- case n if n.endsWith(".class") => MergeStrategy.first
- case n if n.endsWith(".txt") => MergeStrategy.concat
- case n if n.endsWith("NOTICE") => MergeStrategy.concat
- case n => (assembly / assemblyMergeStrategy).value.apply(n)
+ case n if n.endsWith(".class") => MergeStrategy.first
+ case n if n.endsWith(".txt") => MergeStrategy.concat
+ case n if n.endsWith("NOTICE") => MergeStrategy.concat
+ case n if n.endsWith("LICENSE") => MergeStrategy.concat
Review Comment:
Yes please make a separate PR for this. Although it's true that this only effects netty 4 for now, in general of there multiple `LICENSE`'s we do want to concat them (i.e. this isn't just specific to netty 4)
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] He-Pin commented on a diff in pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#discussion_r1285057084
##########
project/SbtMultiJvm.scala:
##########
@@ -372,11 +374,41 @@ object MultiJvmPlugin extends AutoPlugin {
val connectInput = input && index == 0
log.debug("Starting %s for %s".format(jvmName, testClass))
log.debug(" with JVM options: %s".format(allJvmOptions.mkString(" ")))
- (testClass, Jvm.startJvm(javaBin, allJvmOptions, runOptions, jvmLogger, connectInput))
+ val testClass2Process = (testClass, Jvm.startJvm(javaBin, allJvmOptions, runOptions, jvmLogger, connectInput))
+ if (index == 0) {
+ log.debug("%s for %s 's started as `Controller`, waiting before can be connected for clients.".format(jvmName,
+ testClass))
+ val controllerHost = hosts.head
+ val serverPort: Int = Integer.getInteger("multinode.server-port", 4711)
+ waitingBeforeConnectable(controllerHost, serverPort, TimeUnit.SECONDS.toMillis(20L))
+ }
+ testClass2Process
}
processExitCodes(name, processes, log)
}
+ private def waitingBeforeConnectable(host: String, port: Int, timeoutInMillis: Long): Unit = {
+ val inetSocketAddress = new InetSocketAddress(host, port)
+ def telnet(addr: InetSocketAddress, timeout: Int): Boolean = {
+ val socket: Socket = new Socket()
+ try {
+ socket.connect(inetSocketAddress, timeout)
+ socket.isConnected
+ } catch {
+ case _: Exception => false
+ } finally {
+ socket.close()
+ }
+ }
+
+ val startTime = System.currentTimeMillis()
+ var connectivity = false
+ while (!connectivity && (System.currentTimeMillis() - startTime < timeoutInMillis)) {
+ connectivity = telnet(inetSocketAddress, 1000)
+ TimeUnit.MILLISECONDS.sleep(100)
+ }
+ }
Review Comment:
@pjfanning I think it can be running on a slower box now.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] mdedetrich commented on a diff in pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#discussion_r1285185095
##########
cluster/src/multi-jvm/resources/logback-test.xml:
##########
@@ -0,0 +1,13 @@
+<configuration>
+ <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
+ <encoder>
+ <pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern>
+ </encoder>
+ </appender>
+
+ <root level="info">
+ <appender-ref ref="STDOUT" />
+ </root>
+ <logger name="io.netty.util.Recycler" level="ERROR" />
Review Comment:
I thought so, do you mind making this change to `cluster/src/multi-jvm/resources/logback-test.xml` in a separate PR so it can be back-ported separately to Pekko 1.0.x?
I can also do this, and technically speaking you shouldn't even need to update the PR since it would get resolve via a merge.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] He-Pin commented on a diff in pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#discussion_r1285186606
##########
multi-node-testkit/src/main/scala/org/apache/pekko/remote/testconductor/RemoteConnection.scala:
##########
@@ -94,44 +95,88 @@ private[pekko] case object Client extends Role
*/
private[pekko] case object Server extends Role
+/**
+ * INTERNAL API.
+ */
+private[pekko] trait RemoteConnection {
+
+ /**
+ * The channel future associated with this connection.
+ */
+ def channelFuture: ChannelFuture
+
+ /**
+ * Shutdown the connection and release the resources.
+ */
+ def shutdown(): Unit
+}
+
/**
* INTERNAL API.
*/
private[pekko] object RemoteConnection {
- def apply(role: Role, sockaddr: InetSocketAddress, poolSize: Int, handler: ChannelUpstreamHandler): Channel = {
+ def apply(
+ role: Role,
+ sockaddr: InetSocketAddress,
+ poolSize: Int,
+ handler: ChannelInboundHandler): RemoteConnection = {
role match {
case Client =>
- val socketfactory =
- new NioClientSocketChannelFactory(Executors.newCachedThreadPool, Executors.newCachedThreadPool, poolSize)
- val bootstrap = new ClientBootstrap(socketfactory)
- bootstrap.setPipelineFactory(new TestConductorPipelineFactory(handler))
- bootstrap.setOption("tcpNoDelay", true)
- bootstrap.connect(sockaddr).getChannel
+ val bootstrap = new Bootstrap()
+ val eventLoopGroup = new NioEventLoopGroup(poolSize)
+ val cf = bootstrap
+ .group(eventLoopGroup)
+ .channel(classOf[NioSocketChannel])
+ .handler(new TestConductorPipelineFactory(handler))
+ .option[java.lang.Boolean](ChannelOption.TCP_NODELAY, true)
+ .option[java.lang.Boolean](ChannelOption.SO_KEEPALIVE, true)
+ .connect(sockaddr)
+ new RemoteConnection {
+ override def channelFuture: ChannelFuture = cf
+
+ override def shutdown(): Unit = {
+ try {
+ channelFuture.channel().close().sync()
+ eventLoopGroup.shutdownGracefully(1L, 1L, TimeUnit.SECONDS)
+ } catch {
+ case NonFatal(_) => // silence this one to not make tests look like they failed, it's not really critical
+ }
+ }
+ }
+
case Server =>
- val socketfactory =
- new NioServerSocketChannelFactory(Executors.newCachedThreadPool, Executors.newCachedThreadPool, poolSize)
- val bootstrap = new ServerBootstrap(socketfactory)
- bootstrap.setPipelineFactory(new TestConductorPipelineFactory(handler))
- bootstrap.setOption("reuseAddress", !Helpers.isWindows)
- bootstrap.setOption("child.tcpNoDelay", true)
- bootstrap.bind(sockaddr)
+ val bootstrap = new ServerBootstrap()
+ val parentEventLoopGroup = new NioEventLoopGroup(poolSize)
+ val childEventLoopGroup = new NioEventLoopGroup(poolSize)
+ val cf = bootstrap
+ .group(parentEventLoopGroup, childEventLoopGroup)
+ .channel(classOf[NioServerSocketChannel])
+ .childHandler(new TestConductorPipelineFactory(handler))
+ .option[java.lang.Boolean](ChannelOption.SO_REUSEADDR, !Helpers.isWindows)
+ .option[java.lang.Integer](ChannelOption.SO_BACKLOG, 2048)
+ .childOption[java.lang.Boolean](ChannelOption.TCP_NODELAY, true)
+ .childOption[java.lang.Boolean](ChannelOption.SO_KEEPALIVE, true)
+ .bind(sockaddr)
+ .sync()
+
+ new RemoteConnection {
+ override def channelFuture: ChannelFuture = cf
+
+ override def shutdown(): Unit = {
+ try {
+ channelFuture.channel().close().sync()
+ parentEventLoopGroup.shutdownGracefully(1L, 1L, TimeUnit.SECONDS)
Review Comment:
I just updated the code without this, but which may waiting max to 15 seconds. the old code is deprecated.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] He-Pin commented on pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#issuecomment-1666488631
I will udpate the pr wit a `java` ping then.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] pjfanning commented on a diff in pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#discussion_r1285057555
##########
project/SbtMultiJvm.scala:
##########
@@ -372,11 +374,41 @@ object MultiJvmPlugin extends AutoPlugin {
val connectInput = input && index == 0
log.debug("Starting %s for %s".format(jvmName, testClass))
log.debug(" with JVM options: %s".format(allJvmOptions.mkString(" ")))
- (testClass, Jvm.startJvm(javaBin, allJvmOptions, runOptions, jvmLogger, connectInput))
+ val testClass2Process = (testClass, Jvm.startJvm(javaBin, allJvmOptions, runOptions, jvmLogger, connectInput))
+ if (index == 0) {
+ log.debug("%s for %s 's started as `Controller`, waiting before can be connected for clients.".format(jvmName,
+ testClass))
+ val controllerHost = hosts.head
+ val serverPort: Int = Integer.getInteger("multinode.server-port", 4711)
+ waitingBeforeConnectable(controllerHost, serverPort, TimeUnit.SECONDS.toMillis(20L))
+ }
+ testClass2Process
}
processExitCodes(name, processes, log)
}
+ private def waitingBeforeConnectable(host: String, port: Int, timeoutInMillis: Long): Unit = {
+ val inetSocketAddress = new InetSocketAddress(host, port)
+ def telnet(addr: InetSocketAddress, timeout: Int): Boolean = {
+ val socket: Socket = new Socket()
+ try {
+ socket.connect(inetSocketAddress, timeout)
+ socket.isConnected
+ } catch {
+ case _: Exception => false
+ } finally {
+ socket.close()
+ }
+ }
+
+ val startTime = System.currentTimeMillis()
+ var connectivity = false
+ while (!connectivity && (System.currentTimeMillis() - startTime < timeoutInMillis)) {
+ connectivity = telnet(inetSocketAddress, 1000)
+ TimeUnit.MILLISECONDS.sleep(100)
+ }
+ }
Review Comment:
seems like a good solution
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] He-Pin commented on a diff in pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#discussion_r1285007982
##########
project/SbtMultiJvm.scala:
##########
@@ -372,7 +373,13 @@ object MultiJvmPlugin extends AutoPlugin {
val connectInput = input && index == 0
log.debug("Starting %s for %s".format(jvmName, testClass))
log.debug(" with JVM options: %s".format(allJvmOptions.mkString(" ")))
- (testClass, Jvm.startJvm(javaBin, allJvmOptions, runOptions, jvmLogger, connectInput))
+ val testClass2Process = (testClass, Jvm.startJvm(javaBin, allJvmOptions, runOptions, jvmLogger, connectInput))
+ if (index == 0) {
+ log.debug("%s for %s 's started as `Controller`, waiting 2 seconds before start clients.".format(jvmName,
+ testClass))
+ TimeUnit.SECONDS.sleep(2)
Review Comment:
This is needed to get it works on linux and mac.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] He-Pin commented on pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#issuecomment-1666487685
@pjfanning `ping` and `telnet` is not available on Windows, then I need to implement another `telnet` in java to test the controller is started up.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] mdedetrich commented on a diff in pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#discussion_r1285177184
##########
project/SbtMultiJvm.scala:
##########
@@ -167,10 +168,11 @@ object MultiJvmPlugin extends AutoPlugin {
// the first class wins just like a classpath
// just concatenate conflicting text files
assembly / assemblyMergeStrategy := {
- case n if n.endsWith(".class") => MergeStrategy.first
- case n if n.endsWith(".txt") => MergeStrategy.concat
- case n if n.endsWith("NOTICE") => MergeStrategy.concat
- case n => (assembly / assemblyMergeStrategy).value.apply(n)
+ case n if n.endsWith(".class") => MergeStrategy.first
+ case n if n.endsWith(".txt") => MergeStrategy.concat
+ case n if n.endsWith("NOTICE") => MergeStrategy.concat
+ case n if n.endsWith("LICENSE") => MergeStrategy.concat
Review Comment:
Although this change was needed for this feature specifically, I think it would be wiser to make this change in a separate PR because in general we should concatenate all `LICENSE` statements.
##########
multi-node-testkit/src/main/scala/org/apache/pekko/remote/testconductor/RemoteConnection.scala:
##########
@@ -94,44 +95,88 @@ private[pekko] case object Client extends Role
*/
private[pekko] case object Server extends Role
+/**
+ * INTERNAL API.
+ */
+private[pekko] trait RemoteConnection {
+
+ /**
+ * The channel future associated with this connection.
+ */
+ def channelFuture: ChannelFuture
+
+ /**
+ * Shutdown the connection and release the resources.
+ */
+ def shutdown(): Unit
+}
+
/**
* INTERNAL API.
*/
private[pekko] object RemoteConnection {
- def apply(role: Role, sockaddr: InetSocketAddress, poolSize: Int, handler: ChannelUpstreamHandler): Channel = {
+ def apply(
+ role: Role,
+ sockaddr: InetSocketAddress,
+ poolSize: Int,
+ handler: ChannelInboundHandler): RemoteConnection = {
role match {
case Client =>
- val socketfactory =
- new NioClientSocketChannelFactory(Executors.newCachedThreadPool, Executors.newCachedThreadPool, poolSize)
- val bootstrap = new ClientBootstrap(socketfactory)
- bootstrap.setPipelineFactory(new TestConductorPipelineFactory(handler))
- bootstrap.setOption("tcpNoDelay", true)
- bootstrap.connect(sockaddr).getChannel
+ val bootstrap = new Bootstrap()
+ val eventLoopGroup = new NioEventLoopGroup(poolSize)
+ val cf = bootstrap
+ .group(eventLoopGroup)
+ .channel(classOf[NioSocketChannel])
+ .handler(new TestConductorPipelineFactory(handler))
+ .option[java.lang.Boolean](ChannelOption.TCP_NODELAY, true)
+ .option[java.lang.Boolean](ChannelOption.SO_KEEPALIVE, true)
+ .connect(sockaddr)
+ new RemoteConnection {
+ override def channelFuture: ChannelFuture = cf
+
+ override def shutdown(): Unit = {
+ try {
+ channelFuture.channel().close().sync()
+ eventLoopGroup.shutdownGracefully(1L, 1L, TimeUnit.SECONDS)
Review Comment:
Is it worth it to make these shutdown gracefully constants configurable in typesafe config? As I understand these values are the underlying reasons why the implementation initially didn't work on Mac/Linux and if so I think it makes sense for users to be able to configure this without needing a new release incase there are further issues in the future.
##########
cluster/src/multi-jvm/resources/logback-test.xml:
##########
@@ -0,0 +1,13 @@
+<configuration>
+ <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
+ <encoder>
+ <pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern>
+ </encoder>
+ </appender>
+
+ <root level="info">
+ <appender-ref ref="STDOUT" />
+ </root>
+ <logger name="io.netty.util.Recycler" level="ERROR" />
Review Comment:
Is this something that was needed just because of netty4 or is it generally helpful? If it is generally helpful I would do this change in a separate PR so that we can backport it to Pekko 1.0.x
##########
multi-node-testkit/src/main/scala/org/apache/pekko/remote/testconductor/RemoteConnection.scala:
##########
@@ -94,44 +95,88 @@ private[pekko] case object Client extends Role
*/
private[pekko] case object Server extends Role
+/**
+ * INTERNAL API.
+ */
+private[pekko] trait RemoteConnection {
+
+ /**
+ * The channel future associated with this connection.
+ */
+ def channelFuture: ChannelFuture
+
+ /**
+ * Shutdown the connection and release the resources.
+ */
+ def shutdown(): Unit
+}
+
/**
* INTERNAL API.
*/
private[pekko] object RemoteConnection {
- def apply(role: Role, sockaddr: InetSocketAddress, poolSize: Int, handler: ChannelUpstreamHandler): Channel = {
+ def apply(
+ role: Role,
+ sockaddr: InetSocketAddress,
+ poolSize: Int,
+ handler: ChannelInboundHandler): RemoteConnection = {
role match {
case Client =>
- val socketfactory =
- new NioClientSocketChannelFactory(Executors.newCachedThreadPool, Executors.newCachedThreadPool, poolSize)
- val bootstrap = new ClientBootstrap(socketfactory)
- bootstrap.setPipelineFactory(new TestConductorPipelineFactory(handler))
- bootstrap.setOption("tcpNoDelay", true)
- bootstrap.connect(sockaddr).getChannel
+ val bootstrap = new Bootstrap()
+ val eventLoopGroup = new NioEventLoopGroup(poolSize)
+ val cf = bootstrap
+ .group(eventLoopGroup)
+ .channel(classOf[NioSocketChannel])
+ .handler(new TestConductorPipelineFactory(handler))
+ .option[java.lang.Boolean](ChannelOption.TCP_NODELAY, true)
+ .option[java.lang.Boolean](ChannelOption.SO_KEEPALIVE, true)
+ .connect(sockaddr)
+ new RemoteConnection {
+ override def channelFuture: ChannelFuture = cf
+
+ override def shutdown(): Unit = {
+ try {
+ channelFuture.channel().close().sync()
+ eventLoopGroup.shutdownGracefully(1L, 1L, TimeUnit.SECONDS)
+ } catch {
+ case NonFatal(_) => // silence this one to not make tests look like they failed, it's not really critical
+ }
+ }
+ }
+
case Server =>
- val socketfactory =
- new NioServerSocketChannelFactory(Executors.newCachedThreadPool, Executors.newCachedThreadPool, poolSize)
- val bootstrap = new ServerBootstrap(socketfactory)
- bootstrap.setPipelineFactory(new TestConductorPipelineFactory(handler))
- bootstrap.setOption("reuseAddress", !Helpers.isWindows)
- bootstrap.setOption("child.tcpNoDelay", true)
- bootstrap.bind(sockaddr)
+ val bootstrap = new ServerBootstrap()
+ val parentEventLoopGroup = new NioEventLoopGroup(poolSize)
+ val childEventLoopGroup = new NioEventLoopGroup(poolSize)
+ val cf = bootstrap
+ .group(parentEventLoopGroup, childEventLoopGroup)
+ .channel(classOf[NioServerSocketChannel])
+ .childHandler(new TestConductorPipelineFactory(handler))
+ .option[java.lang.Boolean](ChannelOption.SO_REUSEADDR, !Helpers.isWindows)
+ .option[java.lang.Integer](ChannelOption.SO_BACKLOG, 2048)
+ .childOption[java.lang.Boolean](ChannelOption.TCP_NODELAY, true)
+ .childOption[java.lang.Boolean](ChannelOption.SO_KEEPALIVE, true)
+ .bind(sockaddr)
+ .sync()
+
+ new RemoteConnection {
+ override def channelFuture: ChannelFuture = cf
+
+ override def shutdown(): Unit = {
+ try {
+ channelFuture.channel().close().sync()
+ parentEventLoopGroup.shutdownGracefully(1L, 1L, TimeUnit.SECONDS)
Review Comment:
Same as previous comment about configuration
##########
project/SbtMultiJvm.scala:
##########
@@ -372,11 +374,41 @@ object MultiJvmPlugin extends AutoPlugin {
val connectInput = input && index == 0
log.debug("Starting %s for %s".format(jvmName, testClass))
log.debug(" with JVM options: %s".format(allJvmOptions.mkString(" ")))
- (testClass, Jvm.startJvm(javaBin, allJvmOptions, runOptions, jvmLogger, connectInput))
+ val testClass2Process = (testClass, Jvm.startJvm(javaBin, allJvmOptions, runOptions, jvmLogger, connectInput))
+ if (index == 0) {
+ log.debug("%s for %s 's started as `Controller`, waiting before can be connected for clients.".format(jvmName,
+ testClass))
+ val controllerHost = hosts.head
+ val serverPort: Int = Integer.getInteger("multinode.server-port", 4711)
+ waitingBeforeConnectable(controllerHost, serverPort, TimeUnit.SECONDS.toMillis(20L))
+ }
+ testClass2Process
}
processExitCodes(name, processes, log)
}
+ private def waitingBeforeConnectable(host: String, port: Int, timeoutInMillis: Long): Unit = {
+ val inetSocketAddress = new InetSocketAddress(host, port)
+ def telnet(addr: InetSocketAddress, timeout: Int): Boolean = {
+ val socket: Socket = new Socket()
+ try {
+ socket.connect(inetSocketAddress, timeout)
+ socket.isConnected
+ } catch {
+ case _: Exception => false
+ } finally {
+ socket.close()
+ }
+ }
+
+ val startTime = System.currentTimeMillis()
+ var connectivity = false
+ while (!connectivity && (System.currentTimeMillis() - startTime < timeoutInMillis)) {
Review Comment:
Same with previous comment about constants.
##########
project/SbtMultiJvm.scala:
##########
@@ -372,11 +374,41 @@ object MultiJvmPlugin extends AutoPlugin {
val connectInput = input && index == 0
log.debug("Starting %s for %s".format(jvmName, testClass))
log.debug(" with JVM options: %s".format(allJvmOptions.mkString(" ")))
- (testClass, Jvm.startJvm(javaBin, allJvmOptions, runOptions, jvmLogger, connectInput))
+ val testClass2Process = (testClass, Jvm.startJvm(javaBin, allJvmOptions, runOptions, jvmLogger, connectInput))
+ if (index == 0) {
+ log.debug("%s for %s 's started as `Controller`, waiting before can be connected for clients.".format(jvmName,
+ testClass))
+ val controllerHost = hosts.head
+ val serverPort: Int = Integer.getInteger("multinode.server-port", 4711)
Review Comment:
As with previous comments about magic constants should this be made configurable (although in this specific case since we are dealing with test classes in sbt project build it should be done by cli args as is done elsewhere).
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] mdedetrich commented on a diff in pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#discussion_r1285186007
##########
project/SbtMultiJvm.scala:
##########
@@ -372,7 +373,13 @@ object MultiJvmPlugin extends AutoPlugin {
val connectInput = input && index == 0
log.debug("Starting %s for %s".format(jvmName, testClass))
log.debug(" with JVM options: %s".format(allJvmOptions.mkString(" ")))
- (testClass, Jvm.startJvm(javaBin, allJvmOptions, runOptions, jvmLogger, connectInput))
+ val testClass2Process = (testClass, Jvm.startJvm(javaBin, allJvmOptions, runOptions, jvmLogger, connectInput))
+ if (index == 0) {
+ log.debug("%s for %s 's started as `Controller`, waiting 2 seconds before start clients.".format(jvmName,
+ testClass))
+ TimeUnit.SECONDS.sleep(2)
Review Comment:
Regarding this point, maybe it makes sense to make this a separate issue to look at after this PR gets merged? The reason I am stating this is because the best way to find out if these time based constants are too low is by seeing how often the tests fail in our nightly github CI action and we can then iterate from there.
Of course most ideal would be to ping the netty cluster until its ready but I don't feel its valid to block the PR over this, for me its critical to get people to test this directly (via snpashots/building locally), hopefully in production to see if there aren't any regressions well before a release happens.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] He-Pin commented on a diff in pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#discussion_r1285184562
##########
multi-node-testkit/src/main/scala/org/apache/pekko/remote/testconductor/RemoteConnection.scala:
##########
@@ -94,44 +95,88 @@ private[pekko] case object Client extends Role
*/
private[pekko] case object Server extends Role
+/**
+ * INTERNAL API.
+ */
+private[pekko] trait RemoteConnection {
+
+ /**
+ * The channel future associated with this connection.
+ */
+ def channelFuture: ChannelFuture
+
+ /**
+ * Shutdown the connection and release the resources.
+ */
+ def shutdown(): Unit
+}
+
/**
* INTERNAL API.
*/
private[pekko] object RemoteConnection {
- def apply(role: Role, sockaddr: InetSocketAddress, poolSize: Int, handler: ChannelUpstreamHandler): Channel = {
+ def apply(
+ role: Role,
+ sockaddr: InetSocketAddress,
+ poolSize: Int,
+ handler: ChannelInboundHandler): RemoteConnection = {
role match {
case Client =>
- val socketfactory =
- new NioClientSocketChannelFactory(Executors.newCachedThreadPool, Executors.newCachedThreadPool, poolSize)
- val bootstrap = new ClientBootstrap(socketfactory)
- bootstrap.setPipelineFactory(new TestConductorPipelineFactory(handler))
- bootstrap.setOption("tcpNoDelay", true)
- bootstrap.connect(sockaddr).getChannel
+ val bootstrap = new Bootstrap()
+ val eventLoopGroup = new NioEventLoopGroup(poolSize)
+ val cf = bootstrap
+ .group(eventLoopGroup)
+ .channel(classOf[NioSocketChannel])
+ .handler(new TestConductorPipelineFactory(handler))
+ .option[java.lang.Boolean](ChannelOption.TCP_NODELAY, true)
+ .option[java.lang.Boolean](ChannelOption.SO_KEEPALIVE, true)
+ .connect(sockaddr)
+ new RemoteConnection {
+ override def channelFuture: ChannelFuture = cf
+
+ override def shutdown(): Unit = {
+ try {
+ channelFuture.channel().close().sync()
+ eventLoopGroup.shutdownGracefully(1L, 1L, TimeUnit.SECONDS)
Review Comment:
Noop, The reason it not working because of the `Player` not wanting the `Controller` started in the sbtMultiJvm. the old `shutdown` method is deprecated but still can be used.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] mdedetrich commented on a diff in pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#discussion_r1285185177
##########
project/SbtMultiJvm.scala:
##########
@@ -372,11 +374,41 @@ object MultiJvmPlugin extends AutoPlugin {
val connectInput = input && index == 0
log.debug("Starting %s for %s".format(jvmName, testClass))
log.debug(" with JVM options: %s".format(allJvmOptions.mkString(" ")))
- (testClass, Jvm.startJvm(javaBin, allJvmOptions, runOptions, jvmLogger, connectInput))
+ val testClass2Process = (testClass, Jvm.startJvm(javaBin, allJvmOptions, runOptions, jvmLogger, connectInput))
+ if (index == 0) {
+ log.debug("%s for %s 's started as `Controller`, waiting before can be connected for clients.".format(jvmName,
+ testClass))
+ val controllerHost = hosts.head
+ val serverPort: Int = Integer.getInteger("multinode.server-port", 4711)
+ waitingBeforeConnectable(controllerHost, serverPort, TimeUnit.SECONDS.toMillis(20L))
+ }
+ testClass2Process
}
processExitCodes(name, processes, log)
}
+ private def waitingBeforeConnectable(host: String, port: Int, timeoutInMillis: Long): Unit = {
+ val inetSocketAddress = new InetSocketAddress(host, port)
+ def telnet(addr: InetSocketAddress, timeout: Int): Boolean = {
+ val socket: Socket = new Socket()
+ try {
+ socket.connect(inetSocketAddress, timeout)
+ socket.isConnected
+ } catch {
+ case _: Exception => false
+ } finally {
+ socket.close()
+ }
+ }
+
+ val startTime = System.currentTimeMillis()
+ var connectivity = false
+ while (!connectivity && (System.currentTimeMillis() - startTime < timeoutInMillis)) {
Review Comment:
I will resolve this for now, I don't want to block the PR because of this and if people have issues we can always update this later (its just tests)
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] mdedetrich commented on a diff in pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#discussion_r1285185095
##########
cluster/src/multi-jvm/resources/logback-test.xml:
##########
@@ -0,0 +1,13 @@
+<configuration>
+ <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
+ <encoder>
+ <pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern>
+ </encoder>
+ </appender>
+
+ <root level="info">
+ <appender-ref ref="STDOUT" />
+ </root>
+ <logger name="io.netty.util.Recycler" level="ERROR" />
Review Comment:
I thought so, do you mind making this change in a separate PR so it can be back-ported separately to Pekko 1.0.x?
I can also do this, and technically speaking you shouldn't even need to update the PR since it would get resolve via a merge.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] He-Pin commented on a diff in pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#discussion_r1285186538
##########
project/SbtMultiJvm.scala:
##########
@@ -167,10 +168,11 @@ object MultiJvmPlugin extends AutoPlugin {
// the first class wins just like a classpath
// just concatenate conflicting text files
assembly / assemblyMergeStrategy := {
- case n if n.endsWith(".class") => MergeStrategy.first
- case n if n.endsWith(".txt") => MergeStrategy.concat
- case n if n.endsWith("NOTICE") => MergeStrategy.concat
- case n => (assembly / assemblyMergeStrategy).value.apply(n)
+ case n if n.endsWith(".class") => MergeStrategy.first
+ case n if n.endsWith(".txt") => MergeStrategy.concat
+ case n if n.endsWith("NOTICE") => MergeStrategy.concat
+ case n if n.endsWith("LICENSE") => MergeStrategy.concat
Review Comment:
Should I seperate this to a dedicated PR? but seems like only Netty 4 contains this thing.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] pjfanning commented on pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#issuecomment-1666486002
I agree that this can be merged to pekko main (when it is approved) and eventually released with pekko 1.1.0.
I think that anyone who needs this code before the pekko 1.1.0 can easily create their own jar. It would probably be useful for a pekko-multi-node-netty4-testkit project to be created in GitHub for that project to release jars. The project can be archived after the pekko 1.1.0 release.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] mdedetrich commented on pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#issuecomment-1666792290
I have just run the cluster tests on the latest version of this PR and I can confirm that they all pass, good work! After the comments are addressed the PR looks to be in good shape to me.
I have attached the test log for those curios
[multi-jvm-test-main-6.txt](https://github.com/apache/incubator-pekko/files/12269096/multi-jvm-test-main-6.txt)
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] mdedetrich commented on a diff in pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#discussion_r1285186007
##########
project/SbtMultiJvm.scala:
##########
@@ -372,7 +373,13 @@ object MultiJvmPlugin extends AutoPlugin {
val connectInput = input && index == 0
log.debug("Starting %s for %s".format(jvmName, testClass))
log.debug(" with JVM options: %s".format(allJvmOptions.mkString(" ")))
- (testClass, Jvm.startJvm(javaBin, allJvmOptions, runOptions, jvmLogger, connectInput))
+ val testClass2Process = (testClass, Jvm.startJvm(javaBin, allJvmOptions, runOptions, jvmLogger, connectInput))
+ if (index == 0) {
+ log.debug("%s for %s 's started as `Controller`, waiting 2 seconds before start clients.".format(jvmName,
+ testClass))
+ TimeUnit.SECONDS.sleep(2)
Review Comment:
Regarding this point in general, maybe it makes sense to make this a separate issue to look at after this PR gets merged? The reason I am stating this is because the best way to find out if these time based constants are too low is by seeing how often the tests fail in our nightly github CI action and we can then iterate from there.
Of course most ideal would be to ping the netty cluster until its ready but I don't feel its valid to block the PR over this, for me its critical to get people to test this directly (via snpashots/building locally), hopefully in production to see if there aren't any regressions well before a release happens.
Note that @He-Pin already solved this issue in this specific case, my point was more talking about the other time waiting constants in general.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] He-Pin commented on a diff in pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#discussion_r1285186803
##########
project/SbtMultiJvm.scala:
##########
@@ -372,11 +374,41 @@ object MultiJvmPlugin extends AutoPlugin {
val connectInput = input && index == 0
log.debug("Starting %s for %s".format(jvmName, testClass))
log.debug(" with JVM options: %s".format(allJvmOptions.mkString(" ")))
- (testClass, Jvm.startJvm(javaBin, allJvmOptions, runOptions, jvmLogger, connectInput))
+ val testClass2Process = (testClass, Jvm.startJvm(javaBin, allJvmOptions, runOptions, jvmLogger, connectInput))
+ if (index == 0) {
+ log.debug("%s for %s 's started as `Controller`, waiting before can be connected for clients.".format(jvmName,
+ testClass))
Review Comment:
@mdedetrich This is the real fix which can be a sleep(2000) or like thing. the error was an `CONNECT(2)` from the networking , which I was thing caused by resolver or my code bug.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] He-Pin commented on a diff in pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#discussion_r1285184821
##########
project/SbtMultiJvm.scala:
##########
@@ -372,11 +374,41 @@ object MultiJvmPlugin extends AutoPlugin {
val connectInput = input && index == 0
log.debug("Starting %s for %s".format(jvmName, testClass))
log.debug(" with JVM options: %s".format(allJvmOptions.mkString(" ")))
- (testClass, Jvm.startJvm(javaBin, allJvmOptions, runOptions, jvmLogger, connectInput))
+ val testClass2Process = (testClass, Jvm.startJvm(javaBin, allJvmOptions, runOptions, jvmLogger, connectInput))
+ if (index == 0) {
+ log.debug("%s for %s 's started as `Controller`, waiting before can be connected for clients.".format(jvmName,
+ testClass))
+ val controllerHost = hosts.head
+ val serverPort: Int = Integer.getInteger("multinode.server-port", 4711)
Review Comment:
It's configurable with the `-Dmultinode.server-port`.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] He-Pin commented on a diff in pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#discussion_r1285186803
##########
project/SbtMultiJvm.scala:
##########
@@ -372,11 +374,41 @@ object MultiJvmPlugin extends AutoPlugin {
val connectInput = input && index == 0
log.debug("Starting %s for %s".format(jvmName, testClass))
log.debug(" with JVM options: %s".format(allJvmOptions.mkString(" ")))
- (testClass, Jvm.startJvm(javaBin, allJvmOptions, runOptions, jvmLogger, connectInput))
+ val testClass2Process = (testClass, Jvm.startJvm(javaBin, allJvmOptions, runOptions, jvmLogger, connectInput))
+ if (index == 0) {
+ log.debug("%s for %s 's started as `Controller`, waiting before can be connected for clients.".format(jvmName,
+ testClass))
Review Comment:
@mdedetrich This is the real fix which can be a sleep(2000) or like thing. the error was a `CONNECT(2)` from the networking , which I was thing caused by resolver or my code bug.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] pjfanning commented on pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#issuecomment-1666488489
> @pjfanning `ping` and `telnet` is not available on Windows, then I need to implement another `telnet` in java to test the controller is started up.
When I say ping, I don't mean the Unix command. Wiktionary has this meaning (among many) of the word 'ping'.
A packet which a remote host is expected to echo, thus indicating its presence.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] mdedetrich commented on a diff in pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#discussion_r1285175868
##########
cluster/src/multi-jvm/resources/logback-test.xml:
##########
@@ -0,0 +1,13 @@
+<configuration>
+ <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
+ <encoder>
+ <pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern>
+ </encoder>
+ </appender>
+
+ <root level="info">
+ <appender-ref ref="STDOUT" />
+ </root>
+ <logger name="io.netty.util.Recycler" level="ERROR" />
Review Comment:
Is this something that was needed just because of netty4 or is it generally helpful? If it is generally helpful I would do this change in a separate PR so that we can backport it to Pekko 1.0.x
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] He-Pin commented on a diff in pull request #539: !test Migrate multi node testkit to Netty 4.
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #539:
URL: https://github.com/apache/incubator-pekko/pull/539#discussion_r1285184635
##########
cluster/src/multi-jvm/resources/logback-test.xml:
##########
@@ -0,0 +1,13 @@
+<configuration>
+ <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
+ <encoder>
+ <pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern>
+ </encoder>
+ </appender>
+
+ <root level="info">
+ <appender-ref ref="STDOUT" />
+ </root>
+ <logger name="io.netty.util.Recycler" level="ERROR" />
Review Comment:
It because there are so much netty log when you run testing, netty prient so much log and netty 3 don't have those logging.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org