You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by "AnonHxy (via GitHub)" <gi...@apache.org> on 2023/01/31 03:24:52 UTC
[GitHub] [bookkeeper] AnonHxy opened a new pull request, #3758: Fix ReadEntryProcessor v2 SchedulingDelayStats
AnonHxy opened a new pull request, #3758:
URL: https://github.com/apache/bookkeeper/pull/3758
Descriptions of the changes in this PR:
### Motivation
We registered `ReadEntrySchedulingDelayStats` of `ReadEntryProcessor` as `WriteThreadQueuedLatency` mistakenly, so we need fix it.
### Changes
Register `ReadEntrySchedulingDelayStats` if `ReadEntryProcessor`
--
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@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] codecov-commenter commented on pull request #3758: Fix ReadEntryProcessor v2 SchedulingDelayStats
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #3758:
URL: https://github.com/apache/bookkeeper/pull/3758#issuecomment-1409729091
# [Codecov](https://codecov.io/gh/apache/bookkeeper/pull/3758?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> :exclamation: No coverage uploaded for pull request base (`master@c7cc668`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#section-missing-base-commit).
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #3758 +/- ##
=========================================
Coverage ? 44.28%
Complexity ? 4345
=========================================
Files ? 468
Lines ? 40839
Branches ? 5235
=========================================
Hits ? 18084
Misses ? 20870
Partials ? 1885
```
| Flag | Coverage Δ | |
|---|---|---|
| client | `44.28% <0.00%> (?)` | |
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.
: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@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] hangc0276 commented on pull request #3758: Fix ReadEntryProcessor v2 SchedulingDelayStats
Posted by "hangc0276 (via GitHub)" <gi...@apache.org>.
hangc0276 commented on PR #3758:
URL: https://github.com/apache/bookkeeper/pull/3758#issuecomment-1409882993
@lordcheng10 Please help take a look at this Pr, 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: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] AnonHxy commented on pull request #3758: Fix ReadEntryProcessor v2 SchedulingDelayStats
Posted by "AnonHxy (via GitHub)" <gi...@apache.org>.
AnonHxy commented on PR #3758:
URL: https://github.com/apache/bookkeeper/pull/3758#issuecomment-1429011628
@hangc0276 PTAL
--
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@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] lordcheng10 commented on pull request #3758: Fix ReadEntryProcessor v2 SchedulingDelayStats
Posted by "lordcheng10 (via GitHub)" <gi...@apache.org>.
lordcheng10 commented on PR #3758:
URL: https://github.com/apache/bookkeeper/pull/3758#issuecomment-1413042241
Should the following code also use ParsedAddRequest to judge?
https://github.com/apache/bookkeeper/blob/accaa6966f17cb9bf6c7d74de3deac49b758c01b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PacketProcessorBase.java#L178-L180
--
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@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] AnonHxy commented on pull request #3758: Fix ReadEntryProcessor v2 SchedulingDelayStats
Posted by "AnonHxy (via GitHub)" <gi...@apache.org>.
AnonHxy commented on PR #3758:
URL: https://github.com/apache/bookkeeper/pull/3758#issuecomment-1415295792
@StevenLuMT @hangc0276 PTAL
--
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@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] AnonHxy commented on a diff in pull request #3758: Fix ReadEntryProcessor v2 SchedulingDelayStats
Posted by "AnonHxy (via GitHub)" <gi...@apache.org>.
AnonHxy commented on code in PR #3758:
URL: https://github.com/apache/bookkeeper/pull/3758#discussion_r1091592319
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PacketProcessorBase.java:
##########
@@ -166,8 +166,14 @@ protected void sendResponseAndWait(int rc, Object response, OpStatsLogger statsL
@Override
public void run() {
- requestProcessor.getRequestStats().getWriteThreadQueuedLatency()
- .registerSuccessfulEvent(MathUtils.elapsedNanos(enqueueNanos), TimeUnit.NANOSECONDS);
+ if (this instanceof ReadEntryProcessor) {
Review Comment:
OK. It makes senses to me. This could uniform the code style. Updated @hangc0276
--
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@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] hangc0276 merged pull request #3758: Fix ReadEntryProcessor v2 SchedulingDelayStats
Posted by "hangc0276 (via GitHub)" <gi...@apache.org>.
hangc0276 merged PR #3758:
URL: https://github.com/apache/bookkeeper/pull/3758
--
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@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] AnonHxy commented on pull request #3758: Fix ReadEntryProcessor v2 SchedulingDelayStats
Posted by "AnonHxy (via GitHub)" <gi...@apache.org>.
AnonHxy commented on PR #3758:
URL: https://github.com/apache/bookkeeper/pull/3758#issuecomment-1413062494
> @hangc0276 Should the following code also use ParsedAddRequest to judge? for example: if (request instanceof BookieProtocol. ParsedAddRequest) { requestProcessor.onAddRequestFinish(); }
>
> https://github.com/apache/bookkeeper/blob/accaa6966f17cb9bf6c7d74de3deac49b758c01b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PacketProcessorBase.java#L178-L180
I think it looks like a bug that we shoud use `ParsedAddRequest`. Shall we fix it in this PR or a new PR? WDYT @hangc0276 @lordcheng10
--
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@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] hangc0276 commented on a diff in pull request #3758: Fix ReadEntryProcessor v2 SchedulingDelayStats
Posted by "hangc0276 (via GitHub)" <gi...@apache.org>.
hangc0276 commented on code in PR #3758:
URL: https://github.com/apache/bookkeeper/pull/3758#discussion_r1091534807
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PacketProcessorBase.java:
##########
@@ -166,8 +166,14 @@ protected void sendResponseAndWait(int rc, Object response, OpStatsLogger statsL
@Override
public void run() {
- requestProcessor.getRequestStats().getWriteThreadQueuedLatency()
- .registerSuccessfulEvent(MathUtils.elapsedNanos(enqueueNanos), TimeUnit.NANOSECONDS);
+ if (this instanceof ReadEntryProcessor) {
Review Comment:
Use `request instanceof BookieProtocol.ReadRequest`?
--
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@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org