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.
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
Failed gravity update loses whole DB / configuration #5168
Comments
Can you check your
The The |
looks much rather like a
All of these suggestions are somewhat hacky, the appropriate solution would be catching this error and terminating, possibly even cleaning up behind us a bit (delete already downloaded lists) so the disk won't stay filled to the brim possibly causing issues with system logins. |
It was there. At the time I detected Pi-Hole keps a backup, I already had reissued an update with the most important lists to reduce the downtime as well, so I couldn't use it to restore my config from before. However, for my case, that's not a that big loss (1 user Pi-Hole with well-known adlist synchronized from another instance), it was more important for me to issue that bug report so others don't loose their highly configured Pi-Hole instances. From a developer standpoint (but haven't worked on Pi-Hole before), why is the integrated transaction mechanism of SQLite not used? Looking from the outside, it looks like you're simulating a transaction mechanism as good as possible. (no blame intended, I'm considering there is a valid reason, I'm asking just in case). |
Good question, is this something you might know of @PromoFaux |
To fix this:
The above 2 commands in my experience re-installed Pi-Hole just fine. |
@PromoFaux bumping your notification. I know MM has been eating notifications lately. |
Yeah, that sounds sane. Along with something to |
Can you expand a little bit on that? Is there a difference between that process (I'm not familiar with it) and our current process of creating a new database and then swapping in the new file once things are "ready"? (That may be broken here but it is intended for it to be ready.) The reason for that is to have as little downtime as possible with the resolver. It takes some time to build the database and doing that live will block the DNS resolving process until everything is good to go. |
The way we have now swaps the files so there is zero downtime as FTL will simply stick with the "old" file as the file descriptor stays the same even when renaming a file on disk. Only when the new database is in place, we signal FTL to reopen (= use the newer file) and from then on use the new file |
Database transactions in a nutshell are a way to be able to use a database normally but only you see the changes you apply (and these are not really stored) until you commit them. Also, transactions are nearly the same for all DB systems, so Pi-Hole could begin to support more DB vendors without needing to resort to copying files around. Please correct me if I'm wrong: As I understand it, Pi-Hole currently creates a new DB file, connects to both the old & new one and transfers all config from the old DB to the new DB. This would also be no longer required, as you wouldn't need to touch any data not required to be changed on a gravity update. |
Also, a further point: I now probably understand, why my Pi-Hole lost its data. Because SQLite is a single file DB with no persistent daemon like PostgreSQL etc., it has automatic transactions. I expect that following had happened:
Have I missed something or could that be it?
|
When we first drafted the new database-based gravity, we started with the obvious choice of using a transaction very much in the way you described: BEGIN TRANSACTION;
DROP INDEX idx_gravity;
DELETE FROM gravity;
INSERT INTO gravity ...;
CREATE INDEX idx_gravity ON gravity (domain, adlist_id);
COMMIT; and also something like BEGIN TRANSACTION;
CREATE TABLE gravity_new ...;
INSERT INTO gravity_new ...;
CREATE INDEX idx_gravity_1234´ ON gravity_new (domain, adlist_id);
DROP TABLE IF EXISTS gravity_old;
ALTER TABLE gravity RENAME TO gravity_old;
ALTER TABLE gravity_new RENAME TO gravity;
COMMIT; Hints:
Now we come to the problem: Both approaches above do suffer from locking issues with the database file. I agree that they shouldn't but they still did and resorting to the "two completely independent" files solution we have right now was the only viable solution we found back then. It's been slightly over three years since we changed this so don't ask me for the exact details, but I think it was the (lengthy) creation of the index which lead to (concurrency?) issues with the database getting locked during the index creation. This part of my memory seems to be inline with File Locking And Concurrency In SQLite Version 3 (mind the warning but it is still accurate in our case):
This is exactly the point where the database becomes unavailable for us. FTL would not know which domains to block (unless this information is cached because such a domain was requested recently). The issue here is that this can take up to minutes thinking about users with millions of domains on their lists but working on a Raspberry Pi Zero. As said, resorting to a two-independent-files solution was the only thing we found working reliably and without any downtime at all even on the low-end hardware. Of course, we could start experimenting with switching from Summing this all up, I think the best strategy is to improve the gravity script to handle this situation better. Looking though the script with this problem in mind, I see we handle issues with A well-enough compromise might be to ensure that we have trice the amount of space of the current |
Addendum: I just read the
The One other thing I forgot to mention:
It's not that simple. Pi-hole implemented SQL language extensions such as Furthermore, using SQLite3 in the way Pi-hole does have some performance benefits you will not be able to find with a database server attached over network (not even on |
Part of the issue will be mitigated with #5179 as it reduces the number of here-documents |
And this should further mitigate the littering 600540a |
Okay, I see there are good reasons why transactions are not feasible for Pi-Hole, at most due to restrictions from SQLite I wasn't aware (other DB's should be able to handle reads during index creation, at least it was never a problem for systems I wrote). When suggesting other DB systems, I was mainly referring to PostreSQL, which has feature parity with SQLite, but in the case of subnet_match, they require a different syntax to SQLite's one. I asked about it because I couldn't find any discussion about these issues on GitHub and because normal transactions are way less error prone. Thanks for the explanation. |
FTL has Keep in mind that Pi-hole will run on a single core ARM device with 512M RAM, PSQL isn't that light. |
I think there's a couple of options for how to run The rest of the discussion is just gravy. Do we move the temp storage to Those are all great questions but to prevent wandering from the point, lets pin this down to one specific issue. There is nothing to prevent |
This issue has been mentioned on Pi-hole Userspace. There might be relevant details there: https://discourse.pi-hole.net/t/pihole-tmp-no-space-left-on-device/61797/1 |
The temporary directory for gravity functions is now user configurable. I'm leaving this issue open because it addresses more than just a full |
The latest hotfix now includes a check to prevent empty databases. |
Versions
Platform
Expected behavior
When the update of the gravity fails due to a full
/tmp
filesystem, I expect the update to fail.I expect that Pi-Hole can resume working with the old gravity version.
I also expect that Pi-Hole keeps its configuration (which should be stored in
/etc/pihole
, which had plenty of free storage left, ca. 1.7 GiB).Actual behavior / bug
Pi-Hole losts its full configuration (at least groups and configured adlists) and the "blocked domains" counter dropped to 0. Any restart of Pi-Hole did not restore the old database.
Steps to reproduce
Steps to reproduce the behavior:
/tmp
was sufficient for now)pihole -g
(because of huge adlists, the gravity update "hopefully fails")Debug Token
Screenshots
all screenshots
Before Failed Update
Mind the working list & already added (but not yet downloaded) new adlists:
During Update
After Failed Update
Additional context
I continuously monitored the free storage of my root and temp filesystem during the gravity update. Only the temp filesystem was fulled, not the root one.
If it wasn't clear enough from above: The bug report is not about that the gravity update failed due to a full
/tmp
. The bug report is about that Pi-Hole losts / forgot huge parts of its configuration without being able to restore it (automatically).part of logs during failed gravity update
logs from during failed gravity update
The text was updated successfully, but these errors were encountered: