opendal icon indicating copy to clipboard operation
opendal copied to clipboard

feat: make ConcurrentLimitLayer accept Semaphore

Open PsiACE opened this issue 2 months ago • 7 comments

Which issue does this PR close?

Closes #6591 . also cc @bonsairobo for review

Rationale for this change

Expose the internal semaphore used by ConcurrentLimitLayer so external components can coordinate concurrency with OpenDAL without reinventing their own limiters.

What changes are included in this PR?

  • Changed ConcurrentLimitLayer::new to take an Arc<Semaphore> and added the with_permits helper to preserve the prior ergonomic constructor.
  • Added with_http_semaphore plus a documented with_http_concurrent_limit convenience to allow sharing HTTP semaphores as well.
  • Updated Python, Ruby, and Java bindings (and docs/tests) to use the new API surface.

Are there any user-facing changes?

Yes. Applications can now reuse ConcurrentLimitLayer’s semaphore directly to align their own concurrency controls with OpenDAL.

PsiACE avatar Oct 02 '25 05:10 PsiACE

Ummm I can see the original issue is about using user's tokio Semaphore now. Let me think a bit ...

tisonkun avatar Oct 03 '25 16:10 tisonkun

@bonsairobo do you have a public example how a Semaphore would be shared in this case?

tisonkun avatar Oct 03 '25 16:10 tisonkun

@tisonkun I don't

bonsairobo avatar Oct 03 '25 19:10 bonsairobo

@tisonkun You asked if I have a public example. I don't, but my company has a private repository that would make use of this feature.

it does no benefits to tight our interfaces with a certain sync utils impls.

That's not true. While I agree that there is tight coupling, that doesn't imply that no one uses this type of semaphore.

bonsairobo avatar Oct 08 '25 08:10 bonsairobo

@bonsairobo Thanks for your clarification. Could you elaborate a bit how the semaphore share between different resources and how it benefits your use case?

tisonkun avatar Oct 08 '25 08:10 tisonkun

Could you elaborate a bit how the semaphore share between different resources and how it benefits your use case?

@tisonkun OpenDAL is not the only code in our system that opens files. Processes have an open file limit, which is especially low on MacOS (256). We use a global semaphore to limit open file handles. It only works if all code shares that semaphore, so we need to create it ourselves and feed it into the concurrent limit layer.

bonsairobo avatar Oct 08 '25 17:10 bonsairobo

I'm thinking of provide a trait like we do for other layers like RetryLayer. We can allow users to pass any Semaphore they want in the given layer. Do you want to work on this way? @PsiACE

Xuanwo avatar Oct 17 '25 08:10 Xuanwo

Sorry for the delay — this PR has been open for a while and I lost track. I’ve ported the changes to #7082 for a fresh update.

PsiACE avatar Dec 22 '25 07:12 PsiACE