K8SPXC-1748: add checks before creating functions in collector #2264
+72
−14
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pooknull
requested review from
egegunes,
eleo007,
gkech,
hors,
jvpasinatto,
nmarukovich and
valmiranogueira
as code owners
egegunes
requested changes
Nov 24, 2025
Contributor
egegunes
left a comment
egegunes
left a comment
There was a problem hiding this comment.
@pooknull we need to create these functions in entrypoint too so there won't be any DDL when you enable PiTR on a running cluster.
handling these in the collector is to cover an edge case where users have an old cluster with pitr disabled and they want to enable it after upgrading to v1.19.0. if we just create these functions in entrypoint, that'd create problems.
gkech
reviewed
Nov 24, 2025
Comment on lines
+362
to
+369
| err := p.db.QueryRowContext(ctx, `SELECT 1 FROM mysql.func WHERE name = ? LIMIT 1`, functionName).Scan(&x) | ||
| if err != nil && !errors.Is(err, sql.ErrNoRows) { | ||
| return errors.Wrapf(err, "check if function %s exists", functionName) | ||
| } | ||
| if err == nil { | ||
| log.Printf("function %s already exists", functionName) | ||
| continue | ||
| } |
Contributor
There was a problem hiding this comment.
Given that we are using the IF NOT EXISTS on the subsequent query, is this check regarding the existence of the function necessary?
Contributor
There was a problem hiding this comment.
yes, that's the whole point. CREATE IF NOT EXISTS still counts as DDL
egegunes
requested changes
Nov 25, 2025
cmd/pitr/pxc/pxc.go
Outdated
| _, err = p.db.ExecContext(ctx, "CREATE FUNCTION IF NOT EXISTS get_first_record_timestamp_by_binlog RETURNS INTEGER SONAME 'binlog_utils_udf.so'") | ||
| if err != nil { | ||
| return errors.Wrap(err, "create function get_first_record_timestamp_by_binlog") | ||
| createQ := fmt.Sprintf("CREATE FUNCTION IF NOT EXISTS %s RETURNS %s SONAME 'binlog_utils_udf.so'", functionName, returnType) |
Contributor
There was a problem hiding this comment.
we need to wrap this with SET SESSION wsrep_on = OFF; and SET SESSION wsrep_on = ON;
build/pxc-entrypoint.sh
Outdated
Comment on lines
474
to
478
| echo "CREATE FUNCTION IF NOT EXISTS get_last_record_timestamp_by_binlog RETURNS INTEGER SONAME 'binlog_utils_udf.so'" | "${mysql[@]}" | ||
|
|
||
| echo "CREATE FUNCTION IF NOT EXISTS get_gtid_set_by_binlog RETURNS STRING SONAME 'binlog_utils_udf.so'" | "${mysql[@]}" | ||
|
|
||
| echo "CREATE FUNCTION IF NOT EXISTS get_first_record_timestamp_by_binlog RETURNS INTEGER SONAME 'binlog_utils_udf.so'" | "${mysql[@]}" |
Contributor
There was a problem hiding this comment.
i see a lot of test failures with the error full cluster crash detected. probably these statements are the reason. also do we have these in mysql 5.7?
Contributor
|
@pooknull please add description |
egegunes
requested changes
Nov 25, 2025
build/pxc-entrypoint.sh
Outdated
| fi | ||
| set -x | ||
|
|
||
| if [ "$MYSQL_VERSION" == '8.0' ]; then |
Contributor
There was a problem hiding this comment.
let's check for 8.4 as well
egegunes
approved these changes
Nov 28, 2025
Collaborator
commit: 5484528 |
gkech
approved these changes
Nov 28, 2025
hors
requested changes
Nov 28, 2025
Collaborator
hors
left a comment
hors
left a comment
There was a problem hiding this comment.
@pooknull @egegunes We have a problem with smart-update tests - full cluster crash detected
-----------------------------------------------------------------------------------
wait for running cluster
-----------------------------------------------------------------------------------
++ seq 0 2
+ for i in '$(seq 0 $last_pod)'
+ wait_pod smart-update-pxc-0 480
+ local pod=smart-update-pxc-0
+ local max_retry=480
+ local ns=
++ echo smart-update-pxc-0
++ /usr/bin/sed -E 's/.*-(pxc|proxysql)-[0-9]/\1/'
++ grep -E '^(pxc|proxysql)$'
+ local container=pxc
+ set +o xtrace
pod/smart-update-pxc-0 condition met
waiting for pod/smart-update-pxc-0 to become Ready.full cluster crash detected
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://perconadev.atlassian.net/browse/K8SPXC-1748
DESCRIPTION
Problem:
When PITR is enabled, the collector runs the following DDL statements each time it starts:
CREATE FUNCTION IF NOT EXISTS get_last_record_timestamp_by_binlog RETURNS INTEGER SONAME 'binlog_utils_udf.so'CREATE FUNCTION IF NOT EXISTS get_gtid_set_by_binlog RETURNS STRING SONAME 'binlog_utils_udf.so'CREATE FUNCTION IF NOT EXISTS get_first_record_timestamp_by_binlog RETURNS INTEGER SONAME 'binlog_utils_udf.so'Galera treats it as a replicated DDL operation and engages TOI
Solution:
These statements should be executed in the
pxc-entrypoint. The collector should verify that these functions exist and create them if they do not (as a safeguard).CHECKLIST
Jira
Needs Doc) and QA (Needs QA)?Tests
compare/*-oc.yml)?Config/Logging/Testability