MDEV-35646: Limit pseudo_thread_id to UINT32_MAX

[ParadoxV5]

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

What problem is the patch trying to solve?

Although the my_thread_id type is 64 bits, binlog format specs limits it to 32 bits in practice. (See also: MDEV-35706)

The writable SQL variable pseudo_thread_id didn’t realize this though and had a range of ULONGLONG_MAX (at least UINT64_MAX in C/C++).
It consequentially accepted larger values silently, but only the lower 32 bits of whom gets binlogged; this could lead to inconsistency.

Release Notes

The internal variable pseudo_thread_id is now limited to 32 unsigned bits (max. 4294967295) to match the range of thread_id.

How can this PR be tested?

The MTR tests sys_vars.sysvars_server_notembedded and sys_vars.sysvars_server_embedded covers the SQL variable pseudo_thread_id itself.
I got tangled by our #include struture, so I split my plan to fix the aformentioned inconsistency once and for all to MDEV-35706. Solving this would also remedy any truncation that originates not from the user but from our own 🍝.
I skimmed through git grep pseudo_thread_id and found not much though, so capping the SQL variable should be sufficient (unless the system vars constructor is broken).

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.

[bnestere]

Thanks @ParadoxV5 ! See my small note.

[bnestere]

Looks good to me! I only left one note to extend the code comment a bit. Also remember to extend your git commit message with your reviewer (me 😅)

[bnestere]

Good idea to add a comment here! Though instead of just a link to the MDEV, can your comment also add a bit of context to explain the current situation (i.e. why my_thread_id is uint64, whereas the max value is UINT32_MAX), and also explain the premise of MDEV-35706 as to what is to be done about it.

Just noting too, the old value was UINT_MAX32 and here we switch it to UINT32_MAX -- I don't imagine that would cause any problems (nor do I think we should change it, as that'd entail a dependency on my_global.h, which I don't think we want here).

[ParadoxV5]

Recently I began preferring stdint.h counterparts re. my suggestion MDEV-35460.

(nor do I think we should change it, as that'd entail a dependency on my_global.h, which I don't think we want here)

UINT32_MAX is from stdint.h/cstdint while UINT_MAX32 is from my_global.h, so which one do you mean?

[bnestere]

Right. The old value used UINT_MAX32 from my_global.h, and your patch changes this to UINT32_MAX from the standard library. Just wanted to note that the reference here changed.

Also, another comment that applies to the latest patch (writing it here so there is only one comment to pay attention to and nothing is lost 😄). The new comment:

/**
  The binary log format limits `my_thread_id` to 32 bits in practice (though
  not all `thread_id`s are typed as such, including @ref my_thread_id itself).
  @see MDEV-35706
*/

makes it seem like the binary log is the only reason that the thread id is 32-bits, which may be misleading for whoever does actually end up doing the work to extend the thread id to 64 bits (if ever). I think it should be slightly generalized

The server's thread_id is limited to 32 bits in practice because of hard-coded protocols/formats (e.g. the client-server protocol and binary log) ...

[ParadoxV5]

[…] which may be misleading for whoever does actually end up doing the work to extend the thread id to 64 bits (if ever).

ICYMI, @vaintroub mentioned on a MDEV-35646 comment that MDEV-15086 Return the full 64 bits of thread ID in handshake packet was cancelled shortly after I opened MDEV-35706 Shrink my_thread_id to uint32_t.
The uint32_t type would self-describe the valid range and put this constant-typedef unnecessary.

I’ll generalize MDEV-35706 as well (thanks for the heads-up).

[ParadoxV5]

accidentally rebased onto main 😳😳 fixed
Force-pushed: Only an update to this doc is of interest; the rest is rebasing to 10.5.

[vaintroub]

Hmm, I must have meant something else, since MDEV-15086 Return the full 64 bits of thread ID in handshake packet had a rather short lifespan of 1 day, from 2018-01-26 to 2018-01-27.

[CLAassistant]

All committers have signed the CLA.

[vaintroub]

It could also have been wise to move typedef and #define out of my_pthread.h - because , my_pthread.h has nothing to do with connection ids (traditionally misnamed thread_id). my_pthread.h was for OS threads.