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

MDEV-35638 wsrep_last_{written,seen}_gtid call my_error(ER_OUTOFMEMORY) incorrectly #3696

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

Conversation

grooverdan
Copy link
Member

  • The Jira issue number for this PR is: MDEV-35638

Description

The my_error has a flags argument before the varargs size, which in ER_OUTOFMEMORY is a %d. Pass in the size, corrected for what was attempted to be allocated.

For wsrep_sync_wait_upto we error a size roughly of a gtid, though the allocation that failed in wait_gtid_upto was doing:

"m_wait_map.insert(std::make_pair(seqno, &wait_cond))"

which fails. Rather than working out the exact size, which won't be large and not actionably by the user, doing an approximation instead.

Release Notes

Correct OOM messages if they occured during wsrep_last_{writtent,seen}_gtid functions.

How can this PR be tested?

Not easily, manual memory limited, but the calling convention on the error message is quite clear.

If the changes are not amenable to automated testing, please explain why not and carefully describe how to test manually.

Basing the PR against the correct MariaDB version

  • This is a new feature or a refactoring, and the PR is based against the main branch.
  • This is a bug fix, and the PR is based against the earliest maintained branch in which the bug can be reproduced.

PR quality check

  • I checked the CODING_STANDARDS.md file and my PR conforms to this where appropriate.
  • For any trivial modifications to the PR, I am ok with the reviewer making the changes themselves.

…Y) incorrectly.

The my_error has a flags argument before the varargs size, which in
ER_OUTOFMEMORY is a %d. Pass in the size, corrected for what was attempted
to be allocated.

For wsrep_sync_wait_upto we error a size roughly of a gtid, though the
allocation that failed in wait_gtid_upto was doing:

   "m_wait_map.insert(std::make_pair(seqno, &wait_cond))"

which fails. Rather than working out the exact size, which won't be
large and not actionably by the user, doing an approximation instead.
@grooverdan grooverdan added the MariaDB Foundation Pull requests created by MariaDB Foundation label Dec 12, 2024
@grooverdan grooverdan requested a review from knielsen December 12, 2024 23:59
@grooverdan grooverdan marked this pull request as ready for review December 12, 2024 23:59
Copy link
Contributor

@janlindstrom janlindstrom left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Member

@knielsen knielsen left a comment

Choose a reason for hiding this comment

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

The patch is ok, just one comment you can consider.

But this is too minor for old GA release, let's do 11.4 or latest development branch.

Ok to push after changing destination branch to main/11.4.

Comment on lines 5415 to 5421
else if (wait_gtid_ret == ENOMEM)
{
my_error(ER_OUTOFMEMORY, MYF(0), func_name());
/* length is bogus here, no idea how much it tried */
my_error(ER_OUTOFMEMORY, MYF(0), WSREP_GTID_STR_LEN);
ret= 0;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

You could also use here instead the error ER_OUT_OF_RESOURCES (which doesn't take the size).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MariaDB Foundation Pull requests created by MariaDB Foundation
Development

Successfully merging this pull request may close these issues.

3 participants