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

Timer: Add callback users can override to cleanup on cancellation #1579

Open
He-Pin opened this issue Dec 13, 2024 · 4 comments
Open

Timer: Add callback users can override to cleanup on cancellation #1579

He-Pin opened this issue Dec 13, 2024 · 4 comments

Comments

@He-Pin
Copy link
Member

He-Pin commented Dec 13, 2024

Refs: netty/netty#14571

I think it would be nice to have this in Pekko too

@raboof
Copy link
Member

raboof commented Dec 13, 2024

Which API are you thinking of here? Where do we have timers that support cancellation?

@He-Pin
Copy link
Member Author

He-Pin commented Dec 13, 2024

I mean when a ByteBuf/Resource is submitted to Pekko's scheduler with scheduleOnce, but then the task is canceled, then a callback may be need to release the related resource or return it to the pool.

This is some kind of resource management, and in the current implementation of the pekko stream, we lack the support for this too, where in rxjava/reactor-core , when an element is been dropped, an onDrop callback is been called.

@raboof
Copy link
Member

raboof commented Dec 13, 2024

Gotcha, indeed scheduleOnce already returns a Cancellable which has a cancel() and an isCancelled, but doesn't allow 'listening' to the cancellation.

It doesn't look super easy to provide a way to 'listen' to the cancellation without sacrificing performance (or only sacrifices performance if someone is actually listening), but it might be worth a try.

Alternatively, or as a first step, perhaps we could provide a 'wrapper' that allows listening to the cancellation, but only if the cancellation happens through that wrapper?

@He-Pin
Copy link
Member Author

He-Pin commented Dec 13, 2024

Once the Netty one is merged, I will take a look at pekko's too, we found this because there is a OOM/LEAK in our traffic dispatcher :(

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

No branches or pull requests

2 participants