Skip to content

Conversation

@plampio
Copy link
Contributor

@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
Contributor 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.

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
Contributor 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
Contributor 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.

3 participants