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

Add new file watcher behaviours #1812

Merged
merged 9 commits into from
Mar 5, 2023
Merged

Add new file watcher behaviours #1812

merged 9 commits into from
Mar 5, 2023

Conversation

cbentejac
Copy link
Contributor

@cbentejac cbentejac commented Oct 31, 2022 •
edited
Loading

Description

The file watcher behaviour, prior to this PR, was to check every single chunk's status file every 5 seconds, no matter the current status of the graph or the nodes.

This PR adds a button in the GraphEditor to change the settings of the file watcher, with 3 different modes:

  • Auto-refresh: this is the file watcher's current mode; every single chunk's status file is checked every 5 seconds, with no exception
  • Disabled: the file watcher is simply disabled; the watcher thread is either stopped/never started, and no chunk's file status is checked
  • Minimal auto-refresh: every 5 seconds, the file watcher checks the status files of currently submitted or running chunks; if there are no submitted or running chunks, then the file watcher does not do anything

Additionally, at the start of Meshroom, if no submitter is detected, the file watcher is disabled by default; if a submitter is detected, then the minimal auto-refresh mode is enabled.

Every chunk's file status is also updated when calling "Compute", "Submit" or "Delete Data" in order to ensure that the same chunks cannot be computed/submitted twice from different instances that are not all up-to-date.

Features list

  • Add a button to change the file watcher's mode;
  • Add two different behaviours for the file watcher: disabled and minimal auto-refresh;
  • Force the chunk's status update when "Compute", "Submit" or "Delete Data" is triggered.

Implementation remarks

  • In minimal auto-refresh mode, the thread that periodically checks the status files is still running, whether it has chunks' status files to watch or not. If the list of status files to watch is empty, then it will not do anything, but it will still run. The only mode that prevents the thread from running is the "disabled" mode.
  • The different statuses are represented with an enumeration on the Python side. However, our version of PySide2 does not support the exposition of enumerations to QML (https://bugreports.qt.io/browse/PYSIDE-957), so the enumeration values are sent instead as integers.
  • If the file watcher is in minimal auto-refresh and a graph is opened in two different instances of Meshroom and starts being computed in the first one (or is submitted), the second instance of Meshroom will not be aware of it. The only way to update the status of the chunks will either be to reload the graph or to perform a "Refresh Node Status". However, if the user attempts to compute or submit chunks in the second instance that are being computed / have been submitted in the first one, all the chunks will have their status updated, and Meshroom will not attempt to compute them again.

@cbentejac cbentejac force-pushed the dev/fileWatcherStatus branch from e55c9a5 to 13ba38c Compare November 2, 2022 13:26
@cbentejac cbentejac marked this pull request as ready for review November 2, 2022 13:28
fabiencastan
fabiencastan requested changes Nov 2, 2022
View reviewed changes
meshroom/ui/graph.py Show resolved Hide resolved
@fabiencastan fabiencastan added this to the Meshroom 2022.1.0 milestone Nov 2, 2022
@cbentejac cbentejac force-pushed the dev/fileWatcherStatus branch from 13ba38c to cbabc9f Compare November 3, 2022 09:02
@cbentejac cbentejac force-pushed the dev/fileWatcherStatus branch 2 times, most recently from 7c0ff6c to 8a4f0e5 Compare December 7, 2022 12:06
@fabiencastan
Copy link
Member

Minimal refresh does not update changes on submitted nodes as expected.

@cbentejac cbentejac force-pushed the dev/fileWatcherStatus branch from 8a4f0e5 to de45ddf Compare December 8, 2022 16:36
@cbentejac cbentejac marked this pull request as draft December 8, 2022 16:36
@cbentejac cbentejac marked this pull request as draft December 8, 2022 16:36
@cbentejac cbentejac marked this pull request as draft December 8, 2022 16:36
@cbentejac cbentejac force-pushed the dev/fileWatcherStatus branch from de45ddf to 46dcc36 Compare December 22, 2022 15:58
@cbentejac cbentejac force-pushed the dev/fileWatcherStatus branch from 46dcc36 to ef47c35 Compare February 20, 2023 15:47
@cbentejac
[ui] Add a button to disable/enable the file watcher
84715b5 
By default, the file watcher polls every single node's file every 5
seconds, independently from their status. This commit adds a button
in the GraphEditor to disable or enable the file watcher at will.
@cbentejac
[ui] Add a minimal auto-refresh mode for the file watcher
c3108bc 
The default auto-refresh mode of the file watcher checks, every 5 seconds,
every single chunk's status, which may be overkill. Instead of simply
disabling the file watcher, this commit adds a third option that checks,
every 5 seconds, the status of submitted and running chunks. If no chunk
is currently submitted or running, then the file watcher's thread keeps
on running but does nothing.

Additionally, the file watcher is automatically disabled by default if
Meshroom is started without a submitter.
@cbentejac
[ui] Graph: remove unused Python imports
7c43446
@cbentejac
[ui] Update MaterialIcons.ttf and add "published_with_changes" icon
5b4a9ad
@cbentejac
[ui] Use a third icon for the file watcher's "enabled" status
589ead0 
Each file watcher's status now has its own icon:
- Enabled: "published_with_changes"
- Disabled: "sync_disabled"
- Minimal: "sync"
@cbentejac
[ui] Display file watcher's current status in the MaterialButton tooltip
5156053
@cbentejac
[ui] Do not disable the manual refresh button when computations are o…
31579f3 
…ngoing

Only disable it if it has been clicked by the user and the refresh is not
completed yet.
@cbentejac cbentejac force-pushed the dev/fileWatcherStatus branch from ef47c35 to 31579f3 Compare February 28, 2023 16:08
@cbentejac
[ui] Distinguish monitorable and monitored chunks
5bc269b 
When all the chunks were always monitored, no distinction was needed
between chunks that could be monitored and chunks that were monitored.

Now that the minimal refresh mode exists, the distinction is needed.
In particular, when only some chunks are being monitored, it is necessary
to know precisely which ones are to be able to match correctly the status
files with the polled times.
@cbentejac
[ui] Force nodes' status update before performing a submit/compute/de…
614f831 
…lete

This ensures that each node's status is correct before being computed,
submitted, or before having its data deleted. This is especially useful
when the File Poller is in minimal node, and only monitors the nodes that
are currently submitted or running.

If a graph is being opened in two different instances of Meshroom, and
computations are started on it in one of the instances, the other one will
not be aware of it (as the signals indicating computations have started
will have been emitted in the first instance, so no chunk will be added
to the monitoring in the second one). By forcing the update of the statuses
before actually starting computations or deleting data, we ensure that
there will not be any computational conflicts (same nodes submitted twice
in farm, for example) and that the users can know at all times what they
are doing without manually triggering a refresh.

This is not critical for the "Delete Data" part, as the action can already
be triggered even with there is no data to delete, but is still useful
to keep the displayed nodes as up-to-date with their actual status as
possible.
@cbentejac cbentejac marked this pull request as ready for review March 3, 2023 08:03
@cbentejac
Copy link
Contributor Author

The issue with the updates of some chunks in minimal auto-refresh has been fixed, and a safeguard has been added to prevent submitting or computing the same chunks twice, in cases where a graph might not be up-to-date.

(and the branch has been rebased on develop)

fabiencastan
fabiencastan approved these changes Mar 5, 2023
View reviewed changes
Copy link
Member

@fabiencastan fabiencastan left a comment

Choose a reason for hiding this comment

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

Great work!

@fabiencastan fabiencastan merged commit 63bb3fc into develop Mar 5, 2023
@fabiencastan fabiencastan deleted the dev/fileWatcherStatus branch March 5, 2023 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabiencastan fabiencastan fabiencastan approved these changes

@mugulmd mugulmd Awaiting requested review from mugulmd

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Meshroom 2023.1.0
Development

Successfully merging this pull request may close these issues.
2 participants
@cbentejac @fabiencastan