Skip to content

Conversation

@nocturnalastro
Copy link
Collaborator

@nocturnalastro nocturnalastro commented Nov 27, 2025

The polling interval was cause delays in killing processed by moving into a go routine processes it no longer blocks actions such as stopping the process.

@github-actions
Copy link

Thanks for your PR,
Best regards.

@nocturnalastro nocturnalastro added the ok-to-test ok to test label Nov 27, 2025
go pmc.Poll()
for { // Wait for pmc.parentDS to be updated by Poll
time.Sleep(10 * time.Millisecond)
if pmc.parentDS != nil {
Copy link
Collaborator

@josephdrichard josephdrichard Nov 27, 2025

Choose a reason for hiding this comment

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

If RunPMCExpGetParentDS fails, this will never exit this loop?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Even if the poll fails the expect should pick up a change at some point so it might be delayed but should exit in decent time pratically.

I can add a timeout if you'd like I'll just make sure its so long we are unlikely to hit it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also for context this function is called when cloud-event-proxy start/restarts to emit the clock class. So either we're starting up and will receive/update in not long or will already have it so won't enter this block.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've realised I don't actually need the poll here so I just removed it and left a comment on why we can skip.

The polling interval was cause delays in killing processed
My moving into a go routine processes it no longer blocks
actions such as stopping the process.
@nocturnalastro nocturnalastro force-pushed the pmc_monitor_process_with_goroutine_expect branch from eb0b789 to ea5deff Compare November 28, 2025 13:22
@github-actions github-actions bot removed the ok-to-test ok to test label Nov 28, 2025
@nocturnalastro nocturnalastro added the ok-to-test ok to test label Nov 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test ok to test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants