Race Condition? #303
Replies: 6 comments 10 replies
-
|
Hey! This was reported previously but I couldn't reproduce it: https://github.com/jokob-sk/Pi.Alert/issues/259 Happy for you to try to investigate? Thanks in advance! |
Beta Was this translation helpful? Give feedback.
-
|
I'm not quite sure how to start investigating it. I too tried disabling the scans and making changes and that seemed to help, but I had significant issues getting the scans to actually stop. Watching the logs scroll...is Pholus somehow blocking or being run in a synchronous way? This is going to be a bit awkward to describe but when I started tailing the logs I saw the following: It stayed there with no addition log messages written for a minute or so and spit out the following: You can see that as far as the logging is concerned they all printed at the same second. It appears that once Pholus finished it printed the log message, and completed. Watching it for a bit longer tailing the docker logs appears to consistently happen in bursts, this may be a false lead. Some other odd race like behavior I occasionally see.
|
Beta Was this translation helpful? Give feedback.
-
|
I tried moving the database to an SSD and am still running into this issue. Honestly, and this is intuition speaking, I'm leaning to the problem being the multiple readers/writers to the sqllite database. I know their documentation says that it can handle multiple writers by locking the entire database, but having been attempting to make changes extensively it seems like one of the python processes reads the database, then writes over it with it's state, rather than reading it clean. Like the lock happens in the wrong place or my filesystem/os isn't providing filesystem features for managing writes (https://www.sqlite.org/faq.html#q5). Are you opposed to migrating to PostgressSQL or MySQL as the database in the docker compose? It might handle the backend and frontend simultaneous changes better. It looks like you have most of the database access wrapped so the change shouldn't be too onerous. I think there are two python files which would need modified, and one php file along with the compose file and the dockerfile. |
Beta Was this translation helpful? Give feedback.
-
|
Migrating to a different DB is not something I'm considering as I'm unsure if the issue gets solved that way. Also, it's not that big of an issue and this can be mitigated by running Pholus less frequently. I like the simplicity of a SQLlite deployment as well as backups are more straightforward. If rather solve the underlying issue or build in a delay for the Pholus scan. |
Beta Was this translation helpful? Give feedback.
-
|
Fair enough. As I mentioned I'm struggling to turn off the scanning, and I still see the overwrite occur so I'm not sure that it mitigates by running Pholus less frequently (unless there is a seperate bug that is causing Pholus to run more or less continuously). I believe I have every possible scan and/or plugin disabled and the instance is still pretty much continuously scanning. My current config is at the bottom. The following is some selected lines of logs showing a couple things. 1) the near continuous scanning with no significant delay between entering wait, and starting. 2) each trip through is spending ~60 seconds doing something in between Questions: I'm willing to look into those questions myself, but a little pointer as to where in the code I'm looking for the main loop? How is scheduling done? ##Current Config |
Beta Was this translation helpful? Give feedback.
-
|
Thanks so much for the help! I'd love to dedicate more time to the project, but currently, it's about ~1h a day on average, which is limiting. Pholus configPholus is only currently used for name resolution, not device discovery According to your config, Pholus should now be disabled: I haven't noticed any race condition behavior recently and my setup runs Pholus once a day or once a new device is found. e.g. for Schedule: SCAN_CYCLE_MINUTES=15This setting is ONLY related to arp-scan, pihole, DHCP_ACTIVE scans. (I added it into the description) Development vision 🔮I'm slowly planning to transform all scans into Plugins to make the code more maintainable and readable. That would also mean that these scans should be converted in the future (implemented before the Plugins system was a thing): Arp-Scan, Pholus, DHCP_Leases, NMAP For that, I'm updating Plugin and Settings capabilities so it's easier for me to switch those over. I'm also trying to keep setups, settings, backward compatible. The main.py loop has a non-configurable delay of 5s: time.sleep(5) # wait for N seconds Some more context-providing docs on the app structure: https://github.com/jokob-sk/Pi.Alert/blob/main/docs/DATABASE.md Any help would be greatly appreciated, |
Beta Was this translation helpful? Give feedback.
-
|
I'm also moving a lot of the DB access to the API endpoints so the DB isn't used as frequently as it is right now. |
Beta Was this translation helpful? Give feedback.
-
|
I think this is likely a great call, as at a minimum it'll reduce the number of clients trying to read from the database with their own file handles. |
Beta Was this translation helpful? Give feedback.
-
|
I think I found the race. In device.py update_devices_names. The first thing it does is select all the It then does stuff that can take quite a bit of time, in my case its taking 60 seconds or so. And then without checking to see if the record has been updated in the meantime writes over the top of the records with names that were not found (line 464) with the name Combining this with the 5 second sleep in the main loop means that unless you write in that 5 seconds, any change is going to be overwritten. Situation is worse the more I can work on a patch tonight and submit a pull request. I'm thinking of an approach where rather than validate each record hasn't changed instead only update the records where the name is changing. So |
Beta Was this translation helpful? Give feedback.
-
|
Amazing ❤ Thanks a lot - apologies you had to figure the app loop out yourself - it was my bedtime 😴 |
Beta Was this translation helpful? Give feedback.
-
|
Followup to my own question: |
Beta Was this translation helpful? Give feedback.
-
|
Do you think it would cause too frequent DB access? That's the main reason I incorporated the 5s delay. |
Beta Was this translation helpful? Give feedback.
-
|
Since the DB access would would only happen when the user viewed or saved the preferences (and it's already doing the read) I think that might actually be less DB access than waking up and reading the status every 5 seconds to determine if config has updated. |
Beta Was this translation helpful? Give feedback.
-
|
Alright, I'll give it a go and test it |
Beta Was this translation helpful? Give feedback.
-
|
Alright, I'd have to rewrite the main loop as there is a lot of writes to files (E.g. plugins, logging, etc) which would be killing HDD performance. So for now reverting back to the 5s delay. |
Beta Was this translation helpful? Give feedback.
-
|
Thank you both very much for your hard work on this. I noticed this as well and I thought it was normal that names would revert back to default after scans. |
Beta Was this translation helpful? Give feedback.
-
When editing a device name, the change doesn't seem to persist...maybe even most of the time. The change will say it's successful, jumping back to the devices view will show it as successful, but then a few minutes later I'll refresh and notice that it's changed back to whatever the name was originally detected to be. Every once in a while it'll take...and once that happens it sticks.
I didn't create an Issue for this yet, I wanted to make sure this isn't expected behavior where Pholus always overwrites the name with what it detects or something? But again it sticks sometimes...so maybe not.
Beta Was this translation helpful? Give feedback.
All reactions