Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[#9264] Guarded ping only when stream is ready #9266

Merged
merged 1 commit into from
Oct 14, 2022
Merged

[#9264] Guarded ping only when stream is ready #9266

merged 1 commit into from
Oct 14, 2022

Conversation

smilu97
Copy link
Contributor

@smilu97 smilu97 commented Oct 13, 2022 •
edited
Loading

resolves #9264

@smilu97 smilu97 requested a review from emeroad October 13, 2022 07:52
@smilu97 smilu97 self-assigned this Oct 13, 2022
emeroad
emeroad reviewed Oct 13, 2022
View reviewed changes
...ector/src/main/java/com/navercorp/pinpoint/collector/receiver/grpc/service/AgentService.java Outdated
@@ -91,6 +94,10 @@ public void run() {
public StreamObserver<PPing> pingSession(final StreamObserver<PPing> responseObserver) {
final StreamObserver<PPing> request = new StreamObserver<PPing>() {
private final AtomicBoolean first = new AtomicBoolean(false);
private final Runnable warnNotReady = new ThrottledRunnable(() -> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ThrottledLogger would be simpler.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed ThrottledRunnable :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@emeroad emeroad added the bug label Oct 13, 2022
@emeroad emeroad linked an issue Oct 13, 2022 that may be closed by this pull request
Collector pend too many ping-tries #9264
Closed
@codecov
Copy link

codecov bot commented Oct 13, 2022 •
edited
Loading

Codecov Report

Merging #9266 (324a543) into master (9f1bd6b) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##             master    #9266      +/-   ##
============================================
- Coverage     39.61%   39.60%   -0.01%     
- Complexity    11791    11829      +38     
============================================
  Files          3394     3407      +13     
  Lines         91358    91540     +182     
  Branches      10127    10159      +32     
============================================
+ Hits          36188    36253      +65     
- Misses        52084    52187     +103     
- Partials       3086     3100      +14
Impacted Files Coverage Δ
.../collector/receiver/grpc/service/AgentService.java 0.00% <0.00%> (ø)
...p/pinpoint/rpc/client/WriteFailFutureListener.java 56.25% <0.00%> (-25.00%) ⬇️
...p/pinpoint/rpc/stream/StreamChannelRepository.java 57.89% <0.00%> (-21.06%) ⬇️
...base/interceptor/HbaseClientMethodInterceptor.java 55.00% <0.00%> (-18.34%) ⬇️
...nitor/DefaultDataSourceMonitorRegistryService.java 72.41% <0.00%> (-10.35%) ⬇️
...m/navercorp/pinpoint/rpc/client/ConnectFuture.java 70.83% <0.00%> (-8.34%) ⬇️
...vercorp/pinpoint/rpc/packet/ClientClosePacket.java 46.66% <0.00%> (-6.67%) ⬇️
...per/stat/sampling/sampler/AgentUriStatSampler.java 90.00% <0.00%> (-6.43%) ⬇️
...ler/context/provider/sampler/UrlSamplerConfig.java 70.83% <0.00%> (-4.93%) ⬇️
...instrument/transformer/DefaultHierarchyCaches.java 58.97% <0.00%> (-4.92%) ⬇️
... and 84 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@smilu97 smilu97 changed the title [#9164] Guarded ping only when stream is ready [#9264] Guarded ping only when stream is ready Oct 13, 2022
@smilu97 smilu97 merged commit 587dc5c into pinpoint-apm:master Oct 14, 2022
@smilu97 smilu97 deleted the #9164_sus_ping branch October 14, 2022 07:33
@emeroad emeroad added this to the 2.5.0 milestone Dec 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emeroad emeroad emeroad left review comments

Assignees

@smilu97 smilu97

Labels
bug module:collector
Projects
None yet
Milestone
2.5.0
Development

Successfully merging this pull request may close these issues.

Collector pend too many ping-tries
2 participants
@smilu97 @emeroad