pgq / skytools-legacy

Obsolete, see https://github.com/pgq/ for maintained code.
https://github.com/pgq/
Other
248 stars 86 forks source link

creating leaf sets node location incorrectly #25

Open glenndavy opened 11 years ago

glenndavy commented 11 years ago

when replicating between 2 boxen, passing a provider connection string that has more than just the dbname, fails to register correctly.

For eg on node2:

londiste3 someini.ini create-leaf node2 dbname=my_clone --provider "dbname=my_master host=db-master user=just_me"

creates a node_location entry on my_clone.pgq_node.node_location for node1 of "dbname=my_master".

Similarly (and I'm not sure at what point) node1 has a node2 entry for node_location of "dbname=my_clone".

In order to get things working, I just manually add a step and update the node_location column with correct db connection details.

I'm happy to drill in a bit and see if I can fix this (good chance to learn some python) and shoot you a pull request, however can you confirm my understanding of how this should work is correct?

markokr commented 11 years ago

Both of connect strings should have host= also in connect string. Giving connect string without it is usually a bug. Not always - toy installations in single machine - but on real installaltions its a bug.

So perhaps Londiste should throw error in such cases?

glenndavy commented 11 years ago

Hi Markokr, sorry for my delayed response. well the problem I'm having is not that it doesn't enforce passing in a 'host=' but that when I do pass in a host parameter, it is ignored and so I have to set it my self by updating node_location. So, if the error checking is based on the data written into pgq_node.node_location, not data passed in via --provider, then, yep that works from my pov.

martinmarques commented 11 years ago

This is an actual issue. Same experience here. Created the leaf with the --provider="dbname=mydb host=myhost" but no host added to the provider. Maybe tomorrow I can try to get a patch.

markokr commented 11 years ago

I added error for that case. But then people complained who had testenv in single machine. So I downgraded it to warning:

self.log.warning('No host= in public connect string, bad idea')

Although it could probably be better worded.

andrewsw-janrain commented 10 years ago

seeing this bug too. It's not a matter of whether to warn or not on missing host= in public connection string, but that the public connection string is not faithfully inserted into the database at the time the leaf node is created.

given the following command line:

londiste3 test_slave.ini create-leaf node2 dbname=test --provider="dbname=test host=vm1 user=db_user password=secret"

I expect:

select node_location from pgq.node_location where node_name = 'node1'; 

to return:

dbname=test host=vm1 user=db_user password=secret

but instead it returns just:

dbname=test

and replication fails until I manually fix that up in the database.

edit: note that "node1" is the node name for the provider in the create-leaf call above.

justizin commented 10 years ago

I find it interesting that it's suggested in this thread that skytools should warn on missing host= in the public connection string, while in fact the example configurations all work only on a single machine, leaving all of the interesting not-the-same-machine situations to be resolved by the reader.

markokr commented 10 years ago

Following improvements are now done:

What else should be improved so common user would not get confused?

What else is needed to close this bug?