You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ratis.apache.org by GitBox <gi...@apache.org> on 2022/07/22 01:44:05 UTC
[GitHub] [ratis] leo65535 opened a new pull request, #691: RATIS-1633. Improve the grpc config output log information.
leo65535 opened a new pull request, #691:
URL: https://github.com/apache/ratis/pull/691
## What changes were proposed in this pull request?
When run the rpc server, always output redundant and wrong log, like admin.port、client.port.
```
2022-07-22 09:24:21 INFO RaftServer:46 - raft.rpc.type = GRPC (default)
2022-07-22 09:24:21 INFO GrpcFactory:48 - PERFORMANCE WARNING: useCacheForAllThreads is true that may cause Netty to create a lot garbage objects and, as a result, trigger GC.
It is recommended to disable useCacheForAllThreads by setting -Dorg.apache.ratis.thirdparty.io.netty.allocator.useCacheForAllThreads=false in command line.
2022-07-22 09:24:21 INFO GrpcConfigKeys:46 - raft.grpc.admin.port = -1 (default)
2022-07-22 09:24:21 INFO GrpcConfigKeys:46 - raft.grpc.server.port = 10024 (custom)
2022-07-22 09:24:21 INFO GrpcConfigKeys:46 - raft.grpc.client.port = -1 (default)
2022-07-22 09:24:21 INFO GrpcConfigKeys:46 - raft.grpc.server.port = 10024 (custom)
2022-07-22 09:24:21 INFO GrpcConfigKeys:46 - raft.grpc.server.port = 10024 (custom)
2022-07-22 09:24:21 INFO GrpcService:46 - raft.grpc.message.size.max = 64MB (=67108864) (default)
2022-07-22 09:24:21 INFO RaftServerConfigKeys:46 - raft.server.log.appender.buffer.byte-limit = 4MB (=4194304) (default)
2022-07-22 09:24:21 INFO GrpcService:46 - raft.grpc.flow.control.window = 1MB (=1048576) (default)
```
After patch
```
2022-07-21 17:36:03 INFO RaftServer:46 - raft.rpc.type = GRPC (default)
2022-07-21 17:36:03 INFO GrpcFactory:48 - PERFORMANCE WARNING: useCacheForAllThreads is true that may cause Netty to create a lot garbage objects and, as a result, trigger GC.
It is recommended to disable useCacheForAllThreads by setting -Dorg.apache.ratis.thirdparty.io.netty.allocator.useCacheForAllThreads=false in command line.
2022-07-21 17:36:03 INFO GrpcConfigKeys:46 - raft.grpc.admin.port = 10024 (custom)
2022-07-21 17:36:03 INFO GrpcConfigKeys:46 - raft.grpc.client.port = 10024 (custom)
2022-07-21 17:36:03 INFO GrpcConfigKeys:46 - raft.grpc.server.port = 10024 (custom)
2022-07-21 17:36:03 INFO GrpcService:46 - raft.grpc.message.size.max = 64MB (=67108864) (default)
2022-07-21 17:36:03 INFO RaftServerConfigKeys:46 - raft.server.log.appender.buffer.byte-limit = 4MB (=4194304) (default)
2022-07-21 17:36:03 INFO GrpcService:46 - raft.grpc.flow.control.window = 1MB (=1048576) (default)
```
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/RATIS-1633
## How was this patch tested?
(Please explain how this patch was tested. Ex: unit tests, manual tests)
(If this patch involves UI changes, please attach a screen-shot; otherwise, remove 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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] adoroszlai commented on pull request #691: RATIS-1633. Improve the grpc config output log information.
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on PR #691:
URL: https://github.com/apache/ratis/pull/691#issuecomment-1192407427
> it seems the patch doesn't need to be modified any more
Agreed, I didn't ask for that.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo commented on pull request #691: RATIS-1633. Improve the grpc config output log information.
Posted by GitBox <gi...@apache.org>.
szetszwo commented on PR #691:
URL: https://github.com/apache/ratis/pull/691#issuecomment-1192740215
@leo65535 , it is good to print out the effective value but we should print also the valued set in conf. How about the following?
```
raft.grpc.client.port is set to -1 (default); the effective value is 10024 (=raft.grpc.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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] leo65535 commented on pull request #691: RATIS-1633. Improve the grpc config output log information.
Posted by GitBox <gi...@apache.org>.
leo65535 commented on PR #691:
URL: https://github.com/apache/ratis/pull/691#issuecomment-1192411681
> > it seems the patch doesn't need to be modified any more
>
> Agreed, I didn't ask for that.
Thanks.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] leo65535 commented on pull request #691: RATIS-1633. Improve the grpc config output log information.
Posted by GitBox <gi...@apache.org>.
leo65535 commented on PR #691:
URL: https://github.com/apache/ratis/pull/691#issuecomment-1192315120
Thanks @adoroszlai
> I don't think admin.port = -1 (default) is wrong, but it may be confusing if you don't know about the fallback behavior.
We can add some descriptions to user document.
> There is a difference between a configuration that has explicit custom settings for client/admin ports, and one that has no setting and falls back to the server port value.
Emm, it keeps the same logic as before, it seems not a problem. Or, maybe I don't get your points.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo merged pull request #691: RATIS-1633. Improve the grpc config output log information.
Posted by GitBox <gi...@apache.org>.
szetszwo merged PR #691:
URL: https://github.com/apache/ratis/pull/691
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] leo65535 commented on pull request #691: RATIS-1633. Improve the grpc config output log information.
Posted by GitBox <gi...@apache.org>.
leo65535 commented on PR #691:
URL: https://github.com/apache/ratis/pull/691#issuecomment-1192368799
Thanks @adoroszlai, I've learned most of your points.
You are right, they were logged differently (-1 (default) for admin/client before. But the fallback logic has always existed,
although the default value is logged, but the fallback port is actually used. Just from my side, it seems the patch doesn't need to be modified any more.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo commented on pull request #691: RATIS-1633. Improve the grpc config output log information.
Posted by GitBox <gi...@apache.org>.
szetszwo commented on PR #691:
URL: https://github.com/apache/ratis/pull/691#issuecomment-1193177295
Oops, I accidentally merged this. Let's me file a followup JIRA instead of reverting 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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] adoroszlai commented on pull request #691: RATIS-1633. Improve the grpc config output log information.
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on PR #691:
URL: https://github.com/apache/ratis/pull/691#issuecomment-1192332989
> > There is a difference between a configuration that has explicit custom settings for client/admin ports, and one that has no setting and falls back to the server port value.
>
> Emm, it keeps the same logic as before, it seems not a problem. Or, maybe I don't get your points.
I didn't mean the patch breaks anything.
Consider two different configurations:
```
raft.grpc.server.port=12345
```
and
```
raft.grpc.admin.port=12345
raft.grpc.client.port=12345
raft.grpc.server.port=12345
```
Ratis works the same way with both due to the fallback logic. Prior to the patch they were logged differently (`-1 (default)` for admin/client in the first case, `12345 (custom)` in the second case), but with the patch they both result in the same log output.
Now assume we only change `raft.grpc.server.port` to a different value in both cases. Ratis behavior is different for these two configurations (regardless of the patch), since the first one uses the new port as fallback, while the second one has explicit custom config for admin/client ports.
My point is that this difference in log messages may be useful in some cases. (E.g. if someone else is making the config change and you only see the logs.)
But the new log content is probably better in most cases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo commented on pull request #691: RATIS-1633. Improve the grpc config output log information.
Posted by GitBox <gi...@apache.org>.
szetszwo commented on PR #691:
URL: https://github.com/apache/ratis/pull/691#issuecomment-1193181309
Let's make the message shorter as below
```
raft.grpc.admin.port = 10024 (fallback to raft.grpc.server.port)
```
Filed https://issues.apache.org/jira/browse/RATIS-1637
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org