Skip to content

MDEV-36810 Deduplicate LOG_EVENT_OFFSET #4218

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

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

MDEV-36810 Deduplicate LOG_EVENT_OFFSET #4218

wants to merge 1 commit into from

Conversation

ParadoxV5
Copy link
Contributor

@ParadoxV5 ParadoxV5 commented Jul 23, 2025

Description

LOG_EVENT_OFFSET is a duplicate of EVENT_TYPE_OFFSET and BIN_LOG_HEADER_SIZE depending on the context.

(GitHub Blame shows that it came from a changeset import from before I was born 🧒.)

While there, centralize BIN_LOG_HEADER_SIZE to sql/rpl_constants.h previously from sql/unireg.h and client/mysqlbinlog.cc)

Release Notes

N/A

How can this PR be tested?

See that the change doesn’t cause failures on every testing builder

PR quality check

  • 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.
    • Not exactly a bug, wasn’t it?
  • I checked the CODING_STANDARDS.md file and my PR conforms to this where appropriate.
    • When is “appropriate”?
  • For any trivial modifications to the PR, I am ok with the reviewer making the changes themselves.

@ParadoxV5 ParadoxV5 requested a review from bnestere July 23, 2025 03:22
@ParadoxV5 ParadoxV5 added MariaDB Corporation Replication Patches involved in replication labels Jul 23, 2025
@ParadoxV5
Copy link
Contributor Author

ParadoxV5 commented Jul 23, 2025

sql_yacc.yy fails to find BIN_LOG_HEADER_SIZE for CHANGE MASTER TO in not-HAVE_REPLICATION builds.
Where should I move this #define to, if most places expect to only use it under HAVE_REPLICATION? 🤔

Update: the header a level up seem suitable.

  • sql/rpl_constants.h
    • sql/log_event.h
      • client/mysqlbinlog.cc
    • sql/log.h
      • sql/log.cc
      • sql/sql_class.cc
        • sql/sql_yacc.yy

Our codebase certainly has issues with organizing header content.
What’s a “include what you use”?

sql/sql_repl.cc includes

  • log_event.h
  • rpl_rli.h which includes log_event.h
  • rpl_mi.h which includes rpl_rli.h
  • semisync_slave.h which includes rpl_mi.h

`LOG_EVENT_OFFSET` is a duplicate of `EVENT_TYPE_OFFSET`
and `BIN_LOG_HEADER_SIZE` depending on the context.

While there, centralize `BIN_LOG_HEADER_SIZE` to `sql/rpl_constants.h`
(previously from `sql/unireg.h` and `client/mysqlbinlog.cc`)

Reviewed-by: Brandon Nesterenko <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MariaDB Corporation Replication Patches involved in replication
Development

Successfully merging this pull request may close these issues.

1 participant