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: http workers pass their own id #200

Merged

Conversation

benfdking
Copy link
Collaborator

FIRST COMMIT

Currently, stats are written to the store by the Queue class, and before a Queue instance contains a UUID used to identify the worker.

When a Postgres or Redis Queue is created inside a worker, the ID will be unique to that worker, and the stats are stored as expected. In this case, a Queue instance is created with a unique ID wrapped in a Worker class, which then pushes the stats to the store with the ID from that queue instance, which is unique to that worker.

This is currently not quite the case with HTTP. In an HTTP Proxy scenario, the proxy creates a Queue instance that contains a unique ID for the worker, as is done in the case of Redis and Postgres. When an HTTP worker Queue is created, this instance will contain the unique ID representative of that worker. When, however, the HTTP worker reports its stats, these, rather than being associated with the ID associated with the worker, are related to the ID of the queue instance in the proxy server.

This allows the stats to be attributed to the worker id rather than the id in the proxy.

SECOND COMMIT

In the second commit, I corrected what I think is a semantic error: a queue should not have a worker_id, currently stored under uuid in a Queue instance. The second commit removes this id and puts it inside a Worker instance.

@benfdking benfdking force-pushed the let_remote_workers_print_their_own_stats branch from 997dec0 to 395c5a6 Compare January 8, 2025 11:17
@benfdking benfdking force-pushed the let_remote_workers_print_their_own_stats branch from 395c5a6 to 29eca74 Compare January 8, 2025 15:10
@tobymao tobymao merged commit b7d1760 into tobymao:main Jan 8, 2025
5 checks passed
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

Successfully merging this pull request may close these issues.

3 participants