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

This is the first version of the SR speedup feature for MariaDB 10.7. #195

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

plampio
Copy link

@plampio plampio commented Sep 2, 2022

This the second attempt at this PR.

@plampio plampio requested review from sjaakola and temeo September 2, 2022 08:22
* adding all fragments in the streaming log table for the given
* transaction.
*/
virtual int set_fragments_from_table() = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

This method is never called from wsrep-lib side, so it does not have to be in the service interface.

Copy link
Author

@plampio plampio Sep 5, 2022

Choose a reason for hiding this comment

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

Yes, set_fragments_from_table() method is not unused and hence not needed in wsrep-lib. I will remove it, as you suggested.

* Return the binlog cache for the currently executing
* transaction or a NULL pointer if no such cache exists.
*/
virtual void *get_binlog_cache() = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

This method exists only to pass the void pointer as argument for one of the service interface calls. This could be handled on MariaDB side, by storing the binlog cache pointer or client session THD pointer when constructing Wsrep_storage_service object.

@@ -204,6 +216,7 @@ namespace wsrep
size_t fragment_size_;
size_t unit_counter_;
size_t log_position_;
int sr_store_;
Copy link
Contributor

Choose a reason for hiding this comment

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

This variable could perhaps be moved into storage_service implementation on MariaDB side.

Copy link
Author

Choose a reason for hiding this comment

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

It can not be moved to Wsrep_storage_service on the MariaDB side because the lifetime of a Wsrep_storage_service object is too short: the sr_store value should have the same lifetime as the transaction, but on the master an Wsrep_storage_service object exists only for the duration of the append_fragment() call that adds a fragment to the fragment table. In addition to this, on the slave side the Wsrep_storage_service objects are not used at all for SR transactions.

But maybe there is some other class on the MariaDB side where sr_store could be moved.

transaction. This describes the relationship of this WSREP
transaction and the underlying InnoDB transaction.
*/
enum sr_state
Copy link
Contributor

Choose a reason for hiding this comment

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

SR state handling could perhaps be moved to MariaDB side.

Copy link
Author

Choose a reason for hiding this comment

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

Here we have the same question as above with sr_store. We need to find a suitable place on the MariaDB side with a lifetime that is the same as the lifetime of the transaction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants