Issue with Cloudsync compatibility

Help Community Help
1

So I raised a feature request recently to get the 'group' table renamed as that is actually a sqlite keyword.

For the general user this is completely invisible but becomes an issue for scripts such as pihole-cloudsync (which I use but do not author) as it requires a syntax change for exporting the group table (from the master pihole) and actually breaks importing the table (on the slave piholes).

The feature request didnt get very far and is largely out of scope for any pihole discussions as exporting and importing tables is not required for normal functions / 99.9% of your user base - I get that.

however...

In trying to fix the above issue I have just found various things.

  1. the table export can be made to work by enclosing the group name eg 'group'
  2. the .import function will not work with the table name group no matter what you enclose it in, '' "" `` etc.
  3. The only workaround I have for the above is to modify the drop / import process normally used by pihole-cloudsync to be a drop / import to temp / rename temp but only for the group table.
  4. Whether step 3 works or fails seems to also depend on the version of sqlite installed eg I have one system with 3.27.2 that fails as a slave but another system running 3.16.2 that works as a slave with identical code. And its only the group table causing problems.
  5. My only real solution for this is to manually copy the group name settings between piholes then use pihole-cloudsync for everything else - including all the other group membership details - so it's messy

Again this is hardly a pihole problem - but it could be.

There are various feature requests asking for syncing multiple piholes, possibly running a cluster, possibly syncing databases, possibly doing all sorts of things (that might or might not get implemented) and I feel that you are going to hit issues with coding that, and possibly supporting that especially if sqlite version becomes a factor on whether code works.

So what am I doing ? Well all this is a shout-out as I want the devs to be aware of it. I'm not expecting a fix, I'm not asking for a fix (though it would ne nice...). I just want to save you some time if you are going the route of exporting / importing tables between piholes because you will hit issues.

And this is all because the use of the table name 'group' is problematic

2

Thanks for the report, I'm sure the devs are aware of the group issue.

Most of your text might be better suited in GitHub · Where software is built

4

I have an issue raised there and the results of my testing but there is little that can be done. I'm no sql expert but there seem to be few options to workaround the table name

5

Yes, enclosing works in all our use cases. We'll definitely consider renaming tables for v6.0. The guy that originally picked these names has left the project and I don't see why we have to stick with this.

BTW: Enclosing in "" is the proper way. Single-quotes are meant for strings only. SQLite3 accepts them almost everywhere interchangeably, however, they announced that they will make this more strict in future versions.

6

It should, if it doesn't then raise an issue with the cloudsync author.

7

I agree with @DanSchaper that it should work. Without any escaping. The relevant code is here:

(note the " " escaping).

This is the source code of the most recent SQLite3 release, embedded into FTL.

8

That appears to be the route of the difference then.

The above code uses a looping routine with the INSERT INTO function whereas pihole-cloudsync uses the .IMPORT function of sqlite.

I have no idea if that is the best way of doing a sync between master/slave databases but it seems to be the shortest code. 1 line to drop the table, 1 line to import it.

The .IMPORT function seems very limited in its syntax.

The code used by pihole-cloudsync could be made more complicated to deal with that but it seems a shame that it needs to.

With the table name and the limited syntax of .import it's a case of two 'thats inconvenient' combining to make a 'thats a pita'

9

Here's where I run in to some confusion:

Unless you are the only person using cloudsync then why isn't anyone else seeing this?

11

At the moment the current version of cloudsync does not support groups. The groundwork has been done by the author but it's not there yet.

I run 3 pihole, 2 in a common network and 1 in a remote location and I use groups so I decided to look at the code and implement it for myself. I didnt do this via pull request as I was just experimenting. I also liked how the use of githut to sync could be kept private and yet be reachable by any pihole with a net connection.

I found that implementing the group and client sync was easy to code (thanks to how cloudsync had already been written and the fact that group/client related info is just contained in extra tables) but I also ran into factors that complicated my code.

The table name is one but another is the client info. For example it should not be assumed that piholes being synced for their adlists and group design should also have their client info synced as not all piholes will be in the same location and therefore see the same clients. (not an issue just a design consideration).

As I said, I'm not asking for support on this. It's not even official cloudsync code that I am running but the author has indicated that group sync is coming so I assume he will run into the same issues I have.

If I could code it properly then I would do a pull request but at the moment, with the group table and the syntax / unreliable nature of .import, its a bodge so I dont want to.

I'm just trying to raise awareness of the issue, if you call it that, to save people some dev time further down the line.

12

Well, lets see the code and maybe we can help. Though I wouldn't say that needing a loop instead of a one-liner to be complicated. It truly is just a case of quoting the table names. We don't know what you are doing so we can't look at replicating it or finding out if the issue lies elsewhere.

14

So cloudsync has two basic functions, controlled from the commandline.

The nominated master pihole exports the relevant tables to csv files and uses standard github processes to detect changes and push them to a repository

$SUDO sqlite3 $gravity_db -header -csv "SELECT * FROM adlist" >$ad_list

This is just repeated for all relevant tables and can include anything, adlists, groups, clients it doesnt matter as its just an export. This works flawlessly for all tables but requires a minor syntax change for the group table due to the name.

    $SUDO sqlite3 $gravity_db -header -csv "SELECT * FROM 'group'" >$group_list

The nominated slave(s) use the exact same github processes to detect changes and update the local cache. If changes are detected then the FTL process is stopped, various tables are dropped and then recreated using the sqlite built in .import function and then FTL is restarted.

$SUDO sqlite3 $gravity_db "DROP TABLE adlist;"
$SUDO sqlite3 $gravity_db -header -csv ".import adlist.csv adlist"

This is again repeated for all required tables and this is where you get to choose what to drop/import - such as client data. Tbh this is a great way of doing the sync as the file copy is measured in Kb whereas copying the entire gravity.db can easily be tens of Mb (mine if 79Mb)

This works for all tables except group as the .import function within sqlite refuses to work with that name - as far as I have found anyway. The plain name and quoted name just dont work.

My workaround for this was to make it a 3 step process for the group table

    $SUDO sqlite3 $gravity_db "DROP TABLE 'group';"
    $SUDO sqlite3 $gravity_db -header -csv ".import group.csv group_tmp"
    $SUDO sqlite3 $gravity_db "ALTER TABLE group_tmp RENAME TO 'group';"

However this is unreliable and (apparently) depending on the version of sqlite installed, this either works or errors due to issues with the views within gravity.db looking for main.group after the table has been dropped which prevents the ALTER TABLE function from completing.

Just to be clear, the above is the code as used by cloudsync but I have been testing directly from the sqlite commandline to remove any possible issues introduced by the above syntax and "" quotes etc.

The only table that ever has any problems is the group table. It's not conclusive (but the syntax errors strongly indicate) that is is due to the name of the group. All of the operations listed above work perfectly on any table I have created / imported as long as they use a different name [that isnt also a keyword].

sqlite> ALTER TABLE group_tmp RENAME TO 'group';
Error: error in view vw_whitelist: no such table: main.group

This only happens on some slaves and the only clear difference between them is sqlite version.
I tried dropping the view (just to test) and the error just moved to the next view and only stopped when all views were dropped. I'm not sure what the views do but dropping them just to get the group import working definitely felt the wrong direction to go.

15

What is the output you are seeing when you try this? A syntax error or Usage?

16

The reason why we do a copy instead of an in place update is for performance. Copy, edit and then flip that new database in has very minimal downtime. Edit in place takes the database out of use due to locking for the time it takes to do the full process of manipulating the database.

17
18

'Tbh this is a great way of doing the sync as the file copy is measured in Kb whereas copying the entire gravity.db can easily be tens of Mb (mine if 79Mb)'

I meant this purely for how cloudsync works. It keeps the file size down and allows easy use of github and reduces traffic to remote hosts for the sync. They still refresh the adlists when changes are detected but that's still a lot less traffic.

A copy of the database during pihole -up makes perfect sense.

This is what I get when I try and use the .import command within sqlite. As you can see it fails with group but happily works with group2. The code within sqlite only appears to allow very limited syntax for .import which does not include quotes.

SQLite version 3.27.2 2019-02-25 16:06:06
Enter ".help" for usage hints.
sqlite> .tables
adlist domainlist_by_group vw_gravity
adlist_by_group gravity vw_regex_blacklist
client group vw_regex_whitelist
client_by_group info vw_whitelist
domain_audit vw_adlist
domainlist vw_blacklist

sqlite> drop table 'group';
sqlite> .mode csv

sqlite> .import /usr/local/bin/my-pihole-lists/group.csv group
Error: near "group": syntax error

sqlite> .import /usr/local/bin/my-pihole-lists/group.csv 'group'
Error: near "group": syntax error

sqlite> .import /usr/local/bin/my-pihole-lists/group.csv "group"
Error: near "group": syntax error

sqlite> .tables
adlist domainlist vw_blacklist
adlist_by_group domainlist_by_group vw_gravity
client gravity vw_regex_blacklist
client_by_group info vw_regex_whitelist
domain_audit vw_adlist vw_whitelist
sqlite>

sqlite> .import /usr/local/bin/my-pihole-lists/group.csv group2
sqlite> .tables
adlist domainlist_by_group vw_gravity
adlist_by_group gravity vw_regex_blacklist
client group2 vw_regex_whitelist
client_by_group info vw_whitelist
domain_audit vw_adlist
domainlist vw_blacklist
sqlite>

And then the ALTER TABLE fails (for this version of sqlite)

sqlite> ALTER TABLE group2 RENAME TO 'group';
Error: error in view vw_whitelist: no such table: main.group
sqlite>

21

The code line I posted is the .import function of sqlite:

I'm not quite sure what the problem is here, check the following (test.db didn't exist before):

wget https://cdn.sqlitetutorial.net/wp-content/uploads/2016/05/city.csv
sqlite3 test.db ".import city.csv group"
sqlite3 test.db "SELECT COUNT(*) FROM \"group\""

The last command returned 274 telling me that everything worked as expected?

I see you are using an older version of SQLite3

so I'd first suggest using a more recent version. Try the SQLite3 engine embedded into FTL (pihole-FTL sqlite3), it is much more recent than what you have on your system, e.g.


wget https://cdn.sqlitetutorial.net/wp-content/uploads/2016/05/city.csv
rm test.db
pihole-FTL sqlite3 test.db ".import city.csv group"
pihole-FTL sqlite3 test.db "SELECT COUNT(*) FROM \"group\""

Are you still seeing problems?

24

That works and many thanks.

I was not aware that you could access sqlite within FTL like that, I had assumed that the pihole installer just added sqlite from the distro if not present but using your own inbuilt makes much more sense.

I have modified my version of the cloudsync code to use the FTL sqlite and included the use of " instead of ' to quote the group table (for futureproofing) and all apears to work fine.

One thing I noticed is that using the FTL sqlite the DROP TABLE started to produce a foreign key error so I had to amend the drop commands as follows (this is just FYI). Gotta love the variations from different sqlite versions.

old
$SUDO pihole-FTL sqlite3 $gravity_db "DROP TABLE adlist;"

new
$SUDO pihole-FTL sqlite3 $gravity_db "PRAGMA foreign_keys=0;DROP TABLE adlist;"

This doesnt appear to create issues as I am effectively replacing all the tables on the slaves so they shouldn't get out of sync but I will test further.

25

That's fairly new Update SQLite3 to 3.34.0 by DL6ER · Pull Request #1006 · pi-hole/FTL · GitHub

26

Because what you're doing here is not correct: You are deleting the table adlist even though there are still rows in gravity referencing entries in this table.

You should not delete the parent rows before deleting the child rows. Disabling foreign key constraint enforcement is only silencing the issue, several database function may not work reliably/at all afterwards.

27

Then I am at a bit of a loss as to how multiple piholes can be synced without copying the entire gravity database. Many of the tables and views are interrelated.

Even if I could come up with the correct order of dropping tables I would need to drop tables not directly affected by adlist / group / client changes that are wanting to be synced. And then recreate them using blank tables that are repopulated by pihole -g or full exports which end up being large files to sync.

I also think that the potential problems being highlighted by the foreign key errors are not new, they were always present due to the way that cloudsync works - they just werent being reported because cloudsync was using the distro sqlite not the FTL version.

Another unfortunate aspect of this is that cloudsync syncing groups has now become dependent on the version of FTL installed (or distro sqlite). Previous to this the script would work with any version of 5.x but for groups it requires a version that supports the FTL sqlite shell - which appears to date from Dec-20 so version 5.4 ? (or it requires that the distro version of sqlite be reasonably new).

For example the systems I am running pihole on have sqlite 3.27.2 - these are new builds of debian buster and thats the version of sqlite in the distro.

I notice that someone also edited the title of this thread. Whilst this is a cloudsync issue and relates to an external function that most pihole users dont use it all does come back to the use of group as a table name. None of the other tables generate any of the issues that that one table does - due to its name.