MDEV-8235: Expose check_slave_start_position as SQL function GTID_CHECK_POS()#4956
MDEV-8235: Expose check_slave_start_position as SQL function GTID_CHECK_POS()#4956ayush-jha123 wants to merge 1 commit intoMariaDB:mainfrom
Conversation
gkodinov
added
the
External Contribution
gkodinov
left a comment
gkodinov
left a comment
There was a problem hiding this comment.
Thank you for your contribution! This is a preliminary review.
First, the formalities:
- please keep a single commit, with a commit message in it.
- please make sure all of the buildbot hosts compile and run successfully
Below are some of my thoughts on the functionality and its implementation and test coverage. You can ignore all of the below and take it with the final reviewer. Or address it and have a faster final review with these fixed.
| }; | ||
|
|
||
|
|
||
| class Item_func_gtid_check_pos :public Item_int_func |
There was a problem hiding this comment.
if you are aiming at returning a boolean why not derive from Item_bool_func?
|
|
||
| if (state.load(gtid_str->ptr(), gtid_str->length())) | ||
| { | ||
| if (thd && thd->is_error()) |
There was a problem hiding this comment.
Why clear the error? Why not just return it?
| errormsg= gtid_find_binlog_pos(&state, buf, &until_gtid_state, &until_binlog_state, &found_in_index, &out_start_seek); | ||
| until_binlog_state.free(); | ||
| if (errormsg) | ||
| return 0; |
There was a problem hiding this comment.
This can mean many things. E.g. the log file is absent, cannot be read etc. You will need a proper description of what the returns mean in the JIRA.
| gtid_find_binlog_pos returns an error message if the required | ||
| GTIDs have been purged from the binary logs. | ||
| */ | ||
| bool found_in_index= false; |
There was a problem hiding this comment.
this is where the count (from the index) gets returned. I guess this is good enough to return. But, the actual use also scans the data. Why aren't you doing anything with it? The function can return empty error message but 0 found GTIDs and your function is returning 1 (a match).
| return -1; | ||
| } | ||
|
|
||
| if (state.load(gtid_str->ptr(), gtid_str->length())) |
There was a problem hiding this comment.
This (afaiu) can take many gtids. Not just one. How do you account for that case?
| if ((null_value= args[0]->null_value)) | ||
| return 0; | ||
|
|
||
| int result= gtid_check_pos(current_thd, gtid_str); |
There was a problem hiding this comment.
you get a compile error for embedded here. ifdef the whole thing with HAVE_REPLICATION
There was a problem hiding this comment.
you've forgotten to git add the .result file for this.
| --echo # Get the current GTID position manually and verify it exists | ||
| let $pos= `SELECT @@GLOBAL.gtid_binlog_pos`; | ||
| --replace_result $pos GTID_POS | ||
| eval SELECT GTID_CHECK_POS('$pos'); |
There was a problem hiding this comment.
you're missing a test for an otherwise valid GTID with the right server and domain id that's absent.
ayush-jha123
force-pushed
the
mdev-8235-gtid-pos
branch
from
6d3e4de to
81aa4a1
Compare
This commit implements the GTID_CHECK_POS() SQL function, which allows validating if a given GTID position is reachable within the current set of binary logs. This is useful for external tools or orchestration layers to verify if a node is viable for replication start without actually initiating a slave connection. Features: - Returns 1 if all GTIDs in the requested state are reachable. - Returns 0 if any GTID in the requested state has been purged. - Propagates parsing errors for malformed GTID strings. - Derived from Item_bool_func to ensure native boolean handling. - Protected by HAVE_REPLICATION for safe embedded builds. Implemented via a thin wrapper over the engine-internal gtid_find_binlog_pos() logic.
ayush-jha123
force-pushed
the
mdev-8235-gtid-pos
branch
from
81aa4a1 to
c4377de
Compare
2 participants
I’ve been diving deep into the replication source code for the last two weeks,
investigating MDEV-8235. I noticed this task has been open for a while and realized
how useful it would be for DBAs to have a simple way to check GTID availability
directly from the SQL layer.
After studying how check_slave_start_position() works internally, I’ve implemented
a user-facing version called GTID_CHECK_POS().
What this PR does:
in sql_repl.cc to avoid code duplication.
which makes it robust for automation scripts.
I’m really excited to contribute this to MariaDB and would appreciate feedback
on the implementation.