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

Extend the API with opaque runs #208

Merged
merged 8 commits into from
Sep 23, 2024
Merged

Extend the API with opaque runs #208

merged 8 commits into from
Sep 23, 2024

Conversation

NelsonVides
Copy link
Collaborator

Sometimes a library exposes a very complex API but uses a gen_server
under the hood, and we want to pool this gen_server, but we're not
supposed to explicitly do gen_server:call/3 nor gen_server:cast/2,
but use the API instead. To enable this, we expose the possibility to
run a function callback with a worker once a worker has been found.

@NelsonVides
Copy link
Collaborator Author

Btw, codecov hasn't been getting coverage for a while, as a token is not configured. I don't have access to the codecov org to get one, nor to the GHA secrets here to add one such token, so I need somebody with access to it to fix it 🙏🏽

https://github.com/inaka/worker_pool/actions/runs/10970849472/job/30465212510?pr=208

Codecov report uploader 0.8.0
[2024-09-21T07:46:00.639Z] ['info'] => Project root located at: /home/runner/work/worker_pool/worker_pool
[2024-09-21T07:46:00.640Z] ['info'] -> No token specified or token is empty
[2024-09-21T07:46:00.647Z] ['info'] Searching for coverage files...
[2024-09-21T07:46:00.703Z] ['info'] => Found 1 possible coverage files:
  _build/test/covertool/worker_pool.covertool.xml
[2024-09-21T07:46:00.703Z] ['info'] Processing /home/runner/work/worker_pool/worker_pool/_build/test/covertool/worker_pool.covertool.xml...
[2024-09-21T07:46:00.706Z] ['info'] Detected GitHub Actions as the CI provider.
[2024-09-21T07:46:00.899Z] ['info'] Pinging Codecov: https://codecov.io/upload/v4?package=github-action-3.1.6-uploader-0.8.0&token=*******&branch=run_api&build=10970849472&build_url=https%3A%2F%2Fgithub.com%2Finaka%2Fworker_pool%2Factions%2Fruns%2F10970849472%2Fjob%2F30465212510&commit=6fc0c0646eb5716e8bae2a124b63a2031420bcf0&job=Erlang+CI&pr=208&service=github-actions&slug=inaka%2Fworker_pool&name=&tag=&flags=&parent=
[2024-09-21T07:46:01.078Z] ['error'] There was an error running the uploader: Error uploading to [https://codecov.io:](https://codecov.io/) Error: There was an error fetching the storage URL during POST: 429 - {"message":"Rate limit reached. Please upload with the Codecov repository upload token to resolve issue. Expected time to availability: 1855s."}

[2024-09-21T07:46:01.079Z] ['info'] Codecov will exit with status code 0. If you are expecting a non-zero exit code, please pass in the `-Z` flag

@elbrujohalcon
Copy link
Member

I'm… not super convinced about this, @NelsonVides … can you provide an example of a worker that needs this new approach?

@NelsonVides
Copy link
Collaborator Author

I'm… not super convinced about this, @NelsonVides … can you provide an example of a worker that needs this new approach?

I have two use cases I'm dealing with currently, one is the mysql driver, which hides a gen_server behind a complex API, see how at mysql.erl all the usual SQL operations like transactions or prepared queries are doing a lot of parameter preparation and helpers before doing the final gen_server:call/3.

The other use case I have very often is... supervisors! Supervisors are also gen_servers under the hood, and often I want to have a pool of supervisors, to load-balance spawning children for example, as otherwise a single supervisor would be a bottleneck (see ranch dealing with this some years ago ninenines/ranch#110), so again, you don't gen_server:call/3 a supervisor but use the exposed supervisor API.

Another solution would be to have a wpool:get_worker/2 like the one we already have at get_workers/1, and then do whatever you want with the worker, but that only makes me a bit wary of misuse, a user might do a get_worker/2 with the available_worker strategy and then use the worker much later all while the queue manager thinks it's busy and doesn't give it to anyone else.

@elbrujohalcon
Copy link
Member

I see what you're saying… now… do you mind showing here how you plan to use this new functionality with supervisors, for instance. Maybe even adding that example to the library documentation somehow?

@NelsonVides
Ensure coverage minimum in config
65899e8
@NelsonVides
Save checking timestamp if its not going to be used
eff92c4
@NelsonVides
Fix documentation types
f3ee060
@NelsonVides
Extend the API with opaque runs
eab9100 
Sometimes a library exposes a very complex API but uses a gen_server
under the hood, and we want to pool this gen_server, but we're not
supposed to explicitly do `gen_server:call/3` nor `gen_server:cast/2`,
but use the API instead. To enable this, we expose the possibility to
run a function callback with a worker once a worker has been found.
@NelsonVides
Add a test example of how to start a pool of supervisors
0ef6a31
@NelsonVides
Copy link
Collaborator Author

I see what you're saying… now… do you mind showing here how you plan to use this new functionality with supervisors, for instance. Maybe even adding that example to the library documentation somehow?

Done, added both as an example in the documentation as well as a test :)

elbrujohalcon
elbrujohalcon requested changes Sep 22, 2024
View reviewed changes
Copy link
Member

@elbrujohalcon elbrujohalcon left a comment

Choose a reason for hiding this comment

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

Now I get it!! Thanks, @NelsonVides :)
Just a few considerations/adjustments.

src/wpool_queue_manager.erl Show resolved Hide resolved
src/wpool_process.erl Outdated Show resolved Hide resolved
test/wpool_SUITE.erl Outdated Show resolved Hide resolved
@NelsonVides
Copy link
Collaborator Author

Btw, codecov hasn't been getting coverage for a while, as a token is not configured. I don't have access to the codecov org to get one, nor to the GHA secrets here to add one such token, so I need somebody with access to it to fix it 🙏🏽

https://github.com/inaka/worker_pool/actions/runs/10970849472/job/30465212510?pr=208

Codecov report uploader 0.8.0
[2024-09-21T07:46:00.639Z] ['info'] => Project root located at: /home/runner/work/worker_pool/worker_pool
[2024-09-21T07:46:00.640Z] ['info'] -> No token specified or token is empty
[2024-09-21T07:46:00.647Z] ['info'] Searching for coverage files...
[2024-09-21T07:46:00.703Z] ['info'] => Found 1 possible coverage files:
  _build/test/covertool/worker_pool.covertool.xml
[2024-09-21T07:46:00.703Z] ['info'] Processing /home/runner/work/worker_pool/worker_pool/_build/test/covertool/worker_pool.covertool.xml...
[2024-09-21T07:46:00.706Z] ['info'] Detected GitHub Actions as the CI provider.
[2024-09-21T07:46:00.899Z] ['info'] Pinging Codecov: https://codecov.io/upload/v4?package=github-action-3.1.6-uploader-0.8.0&token=*******&branch=run_api&build=10970849472&build_url=https%3A%2F%2Fgithub.com%2Finaka%2Fworker_pool%2Factions%2Fruns%2F10970849472%2Fjob%2F30465212510&commit=6fc0c0646eb5716e8bae2a124b63a2031420bcf0&job=Erlang+CI&pr=208&service=github-actions&slug=inaka%2Fworker_pool&name=&tag=&flags=&parent=
[2024-09-21T07:46:01.078Z] ['error'] There was an error running the uploader: Error uploading to [https://codecov.io:](https://codecov.io/) Error: There was an error fetching the storage URL during POST: 429 - {"message":"Rate limit reached. Please upload with the Codecov repository upload token to resolve issue. Expected time to availability: 1855s."}

[2024-09-21T07:46:01.079Z] ['info'] Codecov will exit with status code 0. If you are expecting a non-zero exit code, please pass in the `-Z` flag

Btw @elbrujohalcon ☝🏽

@elbrujohalcon
Copy link
Member

Yeah… I saw the codecov thing, @NelsonVides… but I don't have that much spare time this days… Do you mind turning that into an issue I can fix later? Thanks.

@elbrujohalcon elbrujohalcon merged commit cd2e97f into main Sep 23, 2024
1 check passed
@elbrujohalcon elbrujohalcon deleted the run_api branch September 23, 2024 14:48
@elbrujohalcon
Copy link
Member

@NelsonVides should I bake a release right now… or are there other pending things that should be included in it?

@NelsonVides NelsonVides mentioned this pull request Sep 23, 2024
Closed
@NelsonVides
Copy link
Collaborator Author

Yeah… I saw the codecov thing, @NelsonVides… but I don't have that much spare time this days… Do you mind turning that into an issue I can fix later? Thanks.

#210 created :)

@NelsonVides
Copy link
Collaborator Author

@NelsonVides should I bake a release right now… or are there other pending things that should be included in it?

I want to come back to the queue manager as I mentioned, but in the following weeks I'm having a bunch of holidays planned so I can't promise to do it very soon, though I'll try in between. So maybe a release already? Hex packages are easy to push new ones anyway :)

@elbrujohalcon
Copy link
Member

Done! https://hex.pm/packages/worker_pool/6.4.0
But, as usual, I could not publish the docs. I'll add you to the package in hex.pm, @NelsonVides so you can do it yourself :)

@elbrujohalcon
Copy link
Member

Done, you're now allowed to publish this project to hex.pm, @NelsonVides

@NelsonVides
Copy link
Collaborator Author

Done, documentation pushed for both the previous 6.3.0 as well as for this 6.4.0 😁
Thanks for adding me to hex! 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@elbrujohalcon elbrujohalcon elbrujohalcon approved these changes

@paulo-ferraz-oliveira paulo-ferraz-oliveira Awaiting requested review from paulo-ferraz-oliveira

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@NelsonVides @elbrujohalcon