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

Dispatch monitoring messages on tag type, instead of queue name #2168

Merged
merged 6 commits into from
Mar 31, 2022

Conversation

benclifford
Copy link
Collaborator

This means monitoring messages are handled based on their
declared tag type, rather than how they are received by the
database manager.

Specifically, RESOURCE_INFO messages are now handled more
like other messages.

This is groundwork for future PRs which will, for example,
allow remote tasks to deliver their resource messages over
htex's existing channels, rather than via UDP.

Type of change

  • Code maintentance/cleanup

This means monitoring messages are handled based on their
declared tag type, rather than how they are received by the
database manager.

Specifically, RESOURCE_INFO messages are now handled more
like other messages.

This is groundwork for future PRs which will, for example,
allow remote tasks to deliver their resource messages over
htex's existing channels, rather than via UDP.
@benclifford benclifford requested a review from yadudoc December 7, 2021 14:52
@yadudoc yadudoc self-assigned this Feb 10, 2022
Copy link
Member

@yadudoc yadudoc left a comment

Choose a reason for hiding this comment

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

I believe this is correct.

@benclifford benclifford merged commit 4488c0b into master Mar 31, 2022
@benclifford benclifford deleted the benc-monitoring-route-by-tag branch March 31, 2022 18:17
benclifford added a commit that referenced this pull request Aug 16, 2024
The main goals of this PR is to make _migrate_logs_to_internal much more
clearly a message forwarder, rather than a message interpreter.

This follows on from PR #2168 which introduces _dispatch_to_internal to
dispatches messages based on their tag rather than on the queue the message
was received on, and is part of an ongoing series to simplify the queue
and routing structure inside the monitoring router and database code.

Further PRs in preparation (in draft PR #3315) contain further simplifications
building on this PR.

After this PR:

* the database manager will respond to a STOP message on any incoming queue,
vs previously only on the priority queue. This is a consequence of treating
the queues all the same now.

* the database manager will not perform such strong validation of message
structure based on message tag at this point. That's part of expecting the
code to forward messages, not inspect them, with later inspecting code
being the place to care abou structure. This only affects behaviour when
invalid messages are sent.

Related PRs and context:

#3567 changes the monitoring router to be more of a router and
to not inspect and modify certain in-transit messages.

There is a long slow project to regularise queues: PR #2117 makes resource
info messages look like other message so they can be dispatched alongside
other message types.

The priority queue was initially (as I understand it) introduced to attempt
to address a race condition of message order arrival vs SQL database key
constraints. The priority queue is an attempt to force certain messages to
be processed before others (not in the priority queue). However a subsequent
commit in 2019, 0a4b685, introduces a
more robust approach because this priority queue approach does not work and
so is not needed.
benclifford added a commit that referenced this pull request Aug 16, 2024
The main goals of this PR is to make _migrate_logs_to_internal much more
clearly a message forwarder, rather than a message interpreter.

This follows on from PR #2168 which introduces _dispatch_to_internal to
dispatches messages based on their tag rather than on the queue the message
was received on, and is part of an ongoing series to simplify the queue
and routing structure inside the monitoring router and database code.

Further PRs in preparation (in draft PR #3315) contain further simplifications
building on this PR.

After this PR:

* the database manager will respond to a STOP message on any incoming queue,
vs previously only on the priority queue. This is a consequence of treating
the queues all the same now.

* the database manager will not perform such strong validation of message
structure based on message tag at this point. That's part of expecting the
code to forward messages, not inspect them, with later inspecting code
being the place to care abou structure. This only affects behaviour when
invalid messages are sent.

Related PRs and context:

#3567 changes the monitoring router to be more of a router and
to not inspect and modify certain in-transit messages.

There is a long slow project to regularise queues: PR #2117 makes resource
info messages look like other message so they can be dispatched alongside
other message types.

The priority queue was initially (as I understand it) introduced to attempt
to address a race condition of message order arrival vs SQL database key
constraints. The priority queue is an attempt to force certain messages to
be processed before others (not in the priority queue). However a subsequent
commit in 2019, 0a4b685, introduces a
more robust approach because this priority queue approach does not work and
so is not needed.
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.

2 participants