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

History Sync: Use single, distinct DB connection #681

Closed
wants to merge 1 commit into from

Conversation

oxzi
Copy link
Member

@oxzi oxzi commented Feb 28, 2024

This PR is another attempt at fixing #577 by using a single, distinct DB connection for the history sync.

The only database tables with foreign key constraints are found in the history context. By default - and for good reasons - a DB internally consists of a cluster of several database connections. However, if a multi-master database server setup is used, it can - and will - happen that this DB contains sessions to different servers and related queries will be submitted in the wrong order, at least in the eye of some database servers.

To prevent this problem, an extra DB with only one connection is used for the history sync. First, this required some refactoring on icingadb.DB, allowing to clone, copy or re-create a DB.

I have tested this change on top of #679, as it happened sometimes that my local Galera cluster wasn't completely ready yet. Thus, this PR should also be considered for merging.

This PR competes with #665 at fixing #577, as both are trying to solve the same issue. Even though I am not completely satisfied with the solution proposed here, I would like to emphasize that no database-specific commands are used and also no cluster sync is require for all INSERTS, also on unrelated tables, as I wrote before.

Please feel free to test it and review it!

The only database tables with foreign key constraints are found in the
history context. By default - and for good reasons - a DB internally
consists of a cluster of several database connections. However, if a
multi-master database server setup is used, it can - and will - happen
that this DB contains sessions to different servers and related queries
will be submitted in the wrong order, at least in the eye of some
database servers.

To prevent this problem, an extra DB with only one connection is used
for the history sync. First, this required some refactoring on
icingadb.DB, allowing to clone, copy or re-create a DB.
Copy link
Member

@Al2Klimov Al2Klimov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach violates almost every possible connection limits configured by the user and either DoSes load-tests the RDBMS with too much parallel input/time (2 connections instead of 1) or adds a bottleneck (1 connection for all history kinds instead of X).

Also, who says that a connection can't break just after a successful parent table insert and re-establish to the other node for a child table insert? Same effect: FK error.

@oxzi
Copy link
Member Author

oxzi commented Mar 5, 2024

Thanks for your feedback, @Al2Klimov.

This approach violates almost every possible connection limits configured by the user

Yes, this results in $configured_max_connections + 1 connections, which could easily be avoided, if, e.g., the total amount would be reduced by one or, if only one total connection should be used, no special connection would be established. Good catch.

and either DoSes load-tests the RDBMS with too much parallel input/time (2 connections instead of 1) or adds a bottleneck (1 connection for all history kinds instead of X).

Could you please elaborate on this one?
AFAIK, in every kind of replication or multi-master-setup, all transactions are going to be replayed by all nodes.

Also, who says that a connection can't break just after a successful parent table insert and re-establish to the other node for a child table insert? Same effect: FK error.

Well, of course, this might also happen. But, in this case, the timeout might rescue us. However, if we are loosing the connection in between, there might already be an issue with the current database node.

@oxzi oxzi marked this pull request as draft March 6, 2024 09:09
@oxzi
Copy link
Member Author

oxzi commented Mar 6, 2024

As outlined in #577 (comment), I think we should not continue with this PR.

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.

2 participants