fix: missing reload events#3831
Conversation
| Install Flow append-to-file.toml | ||
| Execute Command for i in $(seq 3); do tedge mqtt pub seq/events "$i"; done |
There was a problem hiding this comment.
This might still be flacky, even if okay after 10 runs.
There was a problem hiding this comment.
To avoid that flakiness, we need to wait till the file is modified before doing the grep, right?
Came across this inotifywait tool that makes it easier to do that with a simple command:
inotifywait --timeout 1 --event modify
But it's not available out-of-the-box. The inotify-tools package must be installed first.
There was a problem hiding this comment.
We need to wait for the flow to be fully loaded before sending MQTT messages to the flow input topic.
For that we need to improve tedge flow so status or stats messages are published over MQTT when a flow is loaded.
There was a problem hiding this comment.
We need to wait for the flow to be fully loaded before sending MQTT messages to the flow input topic.
Aah okay. I thought the flakiness was because we're doing the grep on the output file immediately after publishing the message, without giving enough time for the mapper to process that input and write to the file.
For that we need to improve tedge flow so status or stats messages are published over MQTT when a flow is loaded.
Yeah, that'd fix both.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! 🚀 New features to boost your workflow:
|
Robot Results
|
| Install Flow append-to-file.toml | ||
| Execute Command for i in $(seq 3); do tedge mqtt pub seq/events "$i"; done |
There was a problem hiding this comment.
To avoid that flakiness, we need to wait till the file is modified before doing the grep, right?
Came across this inotifywait tool that makes it easier to do that with a simple command:
inotifywait --timeout 1 --event modify
But it's not available out-of-the-box. The inotify-tools package must be installed first.
The test "Appending messages to a file" is flaky, but not only because the flow is exercised too soon (i.e. publishing events just after moving the flow definition under the tedge flow directory). But also, because of the following bugs: - The timer of periodic sources are not properly registered. This error was masked by other flows registering step timers. - The inotify messages are not always leading to a proper reload. The root cause being that depending of the command used to deploy a flow, inotify might wrongly notify that a flow has been modified and not added. Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
The flow actor was trying to be smart, but actually too smart. It was reloading a flow on an modified event only if the file was actually loading. However, inotify might wrongly notify an update for a new file. Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
1ccbc2f to
bcbb61e
Compare
Proposed changes
Applying this suggestion #3805 (review)
highlighted two bugs, previously masked because all the tests were pre-loaded and sharing timers.
Types of changes
Paste Link to the issue
Checklist
just prepare-devonce)just formatas mentioned in CODING_GUIDELINESjust checkas mentioned in CODING_GUIDELINESFurther comments