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.

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

Fix recordPendingSuccess/FailureAndLatency not recorded without timeout #214

Merged

Conversation

HeartPattern
Copy link
Contributor

recordPendingSuccess/FailureAndLatency seems not recorded properly without timeout.

If we call acquire() without timeout, timeoutTask remain as TIMEOUT_DISPOSED since AbstractPool#430 check if pendingAcquireTimeout is non-zero.

Copy link
Member

@violetagg violetagg left a comment

Choose a reason for hiding this comment

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

@HeartPattern Thanks for the PR!

Please update the copyright end year with the following command ./gradlew :spotlessApply

Can you please provide test similar to reactor.pool.CommonPoolTest#recordsPendingCountAndLatencies

reactor-pool/src/main/java/reactor/pool/AbstractPool.java Outdated Show resolved Hide resolved
reactor-pool/src/main/java/reactor/pool/AbstractPool.java Outdated Show resolved Hide resolved
@HeartPattern HeartPattern force-pushed the fix-record-pending-without-timeout branch from f006c81 to e5b61ca Compare May 16, 2024 05:18
Copy link
Member

@violetagg violetagg left a comment

Choose a reason for hiding this comment

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

@reactor/team PTAL

@violetagg violetagg requested a review from a team May 16, 2024 10:32
@pderop
Copy link
Contributor

pderop commented May 16, 2024

@violetagg, can you re-review this ?

Copy link
Member

@violetagg violetagg left a comment

Choose a reason for hiding this comment

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

nice

@violetagg violetagg changed the base branch from main to 1.0.x May 17, 2024 10:32
@violetagg violetagg changed the base branch from 1.0.x to main May 17, 2024 10:32
@violetagg violetagg added the type/bug A general bug label May 17, 2024
@violetagg violetagg added this to the 1.0.6 milestone May 17, 2024
@violetagg violetagg merged commit 2e297a8 into reactor:main May 17, 2024
2 checks passed
@violetagg violetagg changed the title Fix recordPendingSuccess/FailureAndLatency not recorded without timeout Fix recordPendingSuccess/FailureAndLatency not recorded without timeout May 17, 2024
@violetagg
Copy link
Member

I back ported this fix to 1.0.x branch. The fix will be available in 1.0.6 and 1.1.0-M3 releases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants