Skip to content

fix: avoid blocking modify monitor by redundant detect call#4154

Open
wilmerdooley wants to merge 1 commit into
apache:masterfrom
wilmerdooley:oss/issue-3812
Open

fix: avoid blocking modify monitor by redundant detect call#4154
wilmerdooley wants to merge 1 commit into
apache:masterfrom
wilmerdooley:oss/issue-3812

Conversation

@wilmerdooley

@wilmerdooley wilmerdooley commented Jun 11, 2026

Copy link
Copy Markdown

What's changed?

This makes modifyMonitor in MonitorServiceImpl no longer block on a synchronous detection pass. Previously the update path called detectMonitor inline, which runs a synchronous collection on the request thread and could block for a long time, eventually timing out the API even though the save itself succeeded.

Removing that synchronous detect on its own would let the final full-row save persist the status carried on the request, overwriting the live status that the real-time collection path maintains (raised in review). So the change also preserves the existing status before the save, monitor.setStatus(preMonitor.getStatus()), on the common path that both the active and paused branches reach. The collector continues to refresh the status on its next scheduled run.

A unit test (testModifyMonitorPreservesLiveStatus) covers both that a live monitor keeps its status across a modify and that a paused monitor is not flipped.

Resolves #3812

Checklist

  • I have read the Contributing Guide
  • I have written the necessary doc or comment.
  • I have added the necessary unit tests and all cases have passed.

Add or update API

  • I have added the necessary e2e tests and all cases have passed.

Signed-off-by: wilmerdooley <wilmerdooley1@gmail.com>
@wilmerdooley

Copy link
Copy Markdown
Author

Thanks again for the catch. One note on the behavior tradeoff with this approach: dropping the synchronous detect means the status now reflects the last value the real-time collection path wrote and reconverges on the next collect cycle, rather than being re-measured on the spot at edit time. If keeping that instant post-edit refresh matters here, I would be glad to move the detect off the request thread and run it asynchronously instead, which would still keep this preserve line so the save does not race it. Otherwise the preserve-only fix as pushed keeps the status correct, and I am happy to take the async direction if it fits the project better.

@Duansg

Duansg commented Jun 14, 2026

Copy link
Copy Markdown
Member

Otherwise the preserve-only fix as pushed keeps the status correct, and I am happy to take the async direction if it fits the project better.

You're welcome, i don't think asynchronous processing is necessary in this situation; we can simply wait for the next cycle after making the changes.

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

Labels

Projects

None yet

2 participants