Skip to content
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

Add foreign keys to the Mysql-scheme #1896

Merged
merged 2 commits into from
Dec 15, 2015

Conversation

tuxis-ie
Copy link
Contributor

Also, add a .sql file that will create the foreign keys on existing databases. But only AFTER the 3.x_to_3.4.0-scripts are run.

@Habbie
Copy link
Member

Habbie commented Dec 2, 2014

We need to make sure there are no performance/locking implications.

@tuxis-ie
Copy link
Contributor Author

tuxis-ie commented Dec 2, 2014

From what I see on a search on 'mysql foreign key performance implications', UPDATES/INSERTS (as I would expect) might suffer from a foreign key. For SELECTS, it looks like people are saying that foreign keys actually improve performance because of the indexes it needs.

Foreign keys AFAIK only kick in on updates and inserts, to check if the foreign key matches. For selects, the foreign key is ignored.

http://books.google.nl/books?id=IagfgRiKWd4C&lpg=PA282&ots=wZa7nYWtzh&dq=mysql%20foreign%20key%20performance%20implications&hl=nl&pg=PA281#v=onepage&q=mysql%20foreign%20key%20performance%20implications&f=false
http://dba.stackexchange.com/questions/15918/foreign-key-constraints-with-mysql-innodb

@Habbie
Copy link
Member

Habbie commented May 6, 2015

I am happy to merge this right now if it is made optional. Alternatively I will entertain serious benchmark numbers :)

@tuxis-ie
Copy link
Contributor Author

tuxis-ie commented May 6, 2015

I'll happily benchmark stuff. Just let me know what you want benchmarked. :)

@Habbie
Copy link
Member

Habbie commented May 6, 2015

Some things I care about in this context:

  1. performance of a busy slave with tens of thousands of domains
  2. chances of hitting long locks/deadlocks on such a slave because a lot more happens during a transfer now

@tuxis-ie
Copy link
Contributor Author

tuxis-ie commented May 6, 2015

PR Updated to only add a file which you can optionally use to enable foreign keys

@cmouse
Copy link
Contributor

cmouse commented May 6, 2015

Would be nice if you could include the file in test suite, though.

@Habbie
Copy link
Member

Habbie commented May 6, 2015

Not sure about that - would make the test suite deviate from common production setups

@cmouse
Copy link
Contributor

cmouse commented May 6, 2015

On the other paw this does make the tests more stringent and does not enable anything new

@Habbie
Copy link
Member

Habbie commented May 6, 2015

True. No objections. Please help @tuxis-ie out, he doesn't know where to put it!

@Habbie
Copy link
Member

Habbie commented Jun 9, 2015

If you add some docs we can merge this, with or without tests.

@Habbie Habbie modified the milestones: auth-4.0.0, auth-4.1.0 Dec 15, 2015
Habbie added a commit that referenced this pull request Dec 15, 2015
Add foreign keys to the Mysql-scheme
@Habbie Habbie merged commit 9f72252 into PowerDNS:master Dec 15, 2015
@ahupowerdns ahupowerdns modified the milestone: auth-4.1.0 Feb 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants