You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/12/09 08:42:09 UTC
[GitHub] [pinot] 61yao opened a new pull request, #9956: [multistage] [observability] Add debugging log for mailbox service
61yao opened a new pull request, #9956:
URL: https://github.com/apache/pinot/pull/9956
Add debugging log for InMemoryMailboxService and GrpcMailboxService
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] 61yao closed pull request #9956: [multistage] [observability] Add debugging log for mailbox service
Posted by "61yao (via GitHub)" <gi...@apache.org>.
61yao closed pull request #9956: [multistage] [observability] Add debugging log for mailbox service
URL: https://github.com/apache/pinot/pull/9956
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr commented on a diff in pull request #9956: [multistage] [observability] Add debugging log for mailbox service
Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9956:
URL: https://github.com/apache/pinot/pull/9956#discussion_r1047433083
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/mailbox/GrpcMailboxService.java:
##########
@@ -89,20 +92,27 @@ public int getMailboxPort() {
* @param mailboxId the id of the mailbox.
*/
public SendingMailbox<TransferableBlock> getSendingMailbox(MailboxIdentifier mailboxId) {
- return _sendingMailboxMap.computeIfAbsent(mailboxId.toString(), (mId) -> new GrpcSendingMailbox(mId, this));
+ SendingMailbox<TransferableBlock> mailbox =
+ _sendingMailboxMap.computeIfAbsent(mailboxId.toString(), (mId) -> new GrpcSendingMailbox(mId, this));
+ LOGGER.trace("GrpcMailboxService sendingMailBox size:" + _sendingMailboxMap.size());
Review Comment:
add sendingmailbox identifier otherwise the trace is very ambiguous
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr commented on a diff in pull request #9956: [multistage] [observability] Add debugging log for mailbox service
Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9956:
URL: https://github.com/apache/pinot/pull/9956#discussion_r1047432457
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/mailbox/GrpcMailboxService.java:
##########
@@ -89,20 +92,27 @@ public int getMailboxPort() {
* @param mailboxId the id of the mailbox.
*/
public SendingMailbox<TransferableBlock> getSendingMailbox(MailboxIdentifier mailboxId) {
- return _sendingMailboxMap.computeIfAbsent(mailboxId.toString(), (mId) -> new GrpcSendingMailbox(mId, this));
+ SendingMailbox<TransferableBlock> mailbox =
+ _sendingMailboxMap.computeIfAbsent(mailboxId.toString(), (mId) -> new GrpcSendingMailbox(mId, this));
+ LOGGER.trace("GrpcMailboxService sendingMailBox size:" + _sendingMailboxMap.size());
+ return mailbox;
}
/**
* Register a mailbox, mailbox needs to be registered before use.
* @param mailboxId the id of the mailbox.
*/
public ReceivingMailbox<TransferableBlock> getReceivingMailbox(MailboxIdentifier mailboxId) {
- return _receivingMailboxMap.computeIfAbsent(
+ ReceivingMailbox<TransferableBlock> mailbox = _receivingMailboxMap.computeIfAbsent(
mailboxId.toString(), (mId) -> new GrpcReceivingMailbox(mId, this, _gotMailCallback));
+ LOGGER.trace("GrpcMailboxService receiving size:" + _receivingMailboxMap.size());
+ return mailbox;
}
public ManagedChannel getChannel(String mailboxId) {
- return _channelManager.getChannel(Utils.constructChannelId(mailboxId));
+ ManagedChannel channel = _channelManager.getChannel(Utils.constructChannelId(mailboxId));
+ LOGGER.trace("GrpcMailboxService channel manager size:" + _channelManager.size());
Review Comment:
why is this only logging the current size?
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr commented on a diff in pull request #9956: [multistage] [observability] Add debugging log for mailbox service
Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9956:
URL: https://github.com/apache/pinot/pull/9956#discussion_r1047433664
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/mailbox/GrpcMailboxService.java:
##########
@@ -89,20 +92,27 @@ public int getMailboxPort() {
* @param mailboxId the id of the mailbox.
*/
public SendingMailbox<TransferableBlock> getSendingMailbox(MailboxIdentifier mailboxId) {
- return _sendingMailboxMap.computeIfAbsent(mailboxId.toString(), (mId) -> new GrpcSendingMailbox(mId, this));
+ SendingMailbox<TransferableBlock> mailbox =
+ _sendingMailboxMap.computeIfAbsent(mailboxId.toString(), (mId) -> new GrpcSendingMailbox(mId, this));
+ LOGGER.trace("GrpcMailboxService sendingMailBox size:" + _sendingMailboxMap.size());
+ return mailbox;
}
/**
* Register a mailbox, mailbox needs to be registered before use.
* @param mailboxId the id of the mailbox.
*/
public ReceivingMailbox<TransferableBlock> getReceivingMailbox(MailboxIdentifier mailboxId) {
- return _receivingMailboxMap.computeIfAbsent(
+ ReceivingMailbox<TransferableBlock> mailbox = _receivingMailboxMap.computeIfAbsent(
mailboxId.toString(), (mId) -> new GrpcReceivingMailbox(mId, this, _gotMailCallback));
+ LOGGER.trace("GrpcMailboxService receiving size:" + _receivingMailboxMap.size());
Review Comment:
same here. please add mailbox ID to the trace
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #9956: [multistage] [observability] Add debugging log for mailbox service
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #9956:
URL: https://github.com/apache/pinot/pull/9956#issuecomment-1345776122
# [Codecov](https://codecov.io/gh/apache/pinot/pull/9956?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#9956](https://codecov.io/gh/apache/pinot/pull/9956?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (32c059c) into [master](https://codecov.io/gh/apache/pinot/commit/173916e42faee24dc1bb3f3d4de2c30bf2efd7a5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (173916e) will **decrease** coverage by `54.63%`.
> The diff coverage is `89.65%`.
```diff
@@ Coverage Diff @@
## master #9956 +/- ##
=============================================
- Coverage 70.46% 15.82% -54.64%
+ Complexity 5535 176 -5359
=============================================
Files 1982 1933 -49
Lines 106449 104359 -2090
Branches 16131 15918 -213
=============================================
- Hits 75006 16512 -58494
- Misses 26213 86624 +60411
+ Partials 5230 1223 -4007
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `15.82% <89.65%> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/9956?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../mailbox/channel/MailboxContentStreamObserver.java](https://codecov.io/gh/apache/pinot/pull/9956/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvbWFpbGJveC9jaGFubmVsL01haWxib3hDb250ZW50U3RyZWFtT2JzZXJ2ZXIuamF2YQ==) | `63.41% <25.00%> (-6.59%)` | :arrow_down: |
| [...apache/pinot/query/mailbox/GrpcMailboxService.java](https://codecov.io/gh/apache/pinot/pull/9956/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvbWFpbGJveC9HcnBjTWFpbGJveFNlcnZpY2UuamF2YQ==) | `96.29% <100.00%> (+1.55%)` | :arrow_up: |
| [...he/pinot/query/mailbox/InMemoryMailboxService.java](https://codecov.io/gh/apache/pinot/pull/9956/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvbWFpbGJveC9Jbk1lbW9yeU1haWxib3hTZXJ2aWNlLmphdmE=) | `100.00% <100.00%> (ø)` | |
| [...he/pinot/query/mailbox/InMemorySendingMailbox.java](https://codecov.io/gh/apache/pinot/pull/9956/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvbWFpbGJveC9Jbk1lbW9yeVNlbmRpbmdNYWlsYm94LmphdmE=) | `70.58% <100.00%> (+6.30%)` | :arrow_up: |
| [...he/pinot/query/mailbox/channel/ChannelManager.java](https://codecov.io/gh/apache/pinot/pull/9956/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvbWFpbGJveC9jaGFubmVsL0NoYW5uZWxNYW5hZ2VyLmphdmE=) | `100.00% <100.00%> (ø)` | |
| [...src/main/java/org/apache/pinot/sql/FilterKind.java](https://codecov.io/gh/apache/pinot/pull/9956/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcWwvRmlsdGVyS2luZC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ain/java/org/apache/pinot/core/data/table/Key.java](https://codecov.io/gh/apache/pinot/pull/9956/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL0tleS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...in/java/org/apache/pinot/spi/utils/BytesUtils.java](https://codecov.io/gh/apache/pinot/pull/9956/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQnl0ZXNVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...n/java/org/apache/pinot/core/data/table/Table.java](https://codecov.io/gh/apache/pinot/pull/9956/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL1RhYmxlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/data/table/Record.java](https://codecov.io/gh/apache/pinot/pull/9956/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL1JlY29yZC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1519 more](https://codecov.io/gh/apache/pinot/pull/9956/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr commented on pull request #9956: [multistage] [observability] Add debugging log for mailbox service
Posted by GitBox <gi...@apache.org>.
walterddr commented on PR #9956:
URL: https://github.com/apache/pinot/pull/9956#issuecomment-1372521174
@61yao could we address the comments and rebase 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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr commented on a diff in pull request #9956: [multistage] [observability] Add debugging log for mailbox service
Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9956:
URL: https://github.com/apache/pinot/pull/9956#discussion_r1047433664
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/mailbox/GrpcMailboxService.java:
##########
@@ -89,20 +92,27 @@ public int getMailboxPort() {
* @param mailboxId the id of the mailbox.
*/
public SendingMailbox<TransferableBlock> getSendingMailbox(MailboxIdentifier mailboxId) {
- return _sendingMailboxMap.computeIfAbsent(mailboxId.toString(), (mId) -> new GrpcSendingMailbox(mId, this));
+ SendingMailbox<TransferableBlock> mailbox =
+ _sendingMailboxMap.computeIfAbsent(mailboxId.toString(), (mId) -> new GrpcSendingMailbox(mId, this));
+ LOGGER.trace("GrpcMailboxService sendingMailBox size:" + _sendingMailboxMap.size());
+ return mailbox;
}
/**
* Register a mailbox, mailbox needs to be registered before use.
* @param mailboxId the id of the mailbox.
*/
public ReceivingMailbox<TransferableBlock> getReceivingMailbox(MailboxIdentifier mailboxId) {
- return _receivingMailboxMap.computeIfAbsent(
+ ReceivingMailbox<TransferableBlock> mailbox = _receivingMailboxMap.computeIfAbsent(
mailboxId.toString(), (mId) -> new GrpcReceivingMailbox(mId, this, _gotMailCallback));
+ LOGGER.trace("GrpcMailboxService receiving size:" + _receivingMailboxMap.size());
Review Comment:
same here and others: please add mailbox ID to the trace
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org