canal: add transactional position sync

[lance6716]

Fixes #1090.

Summary

• Add Config.TransactionalPosSync as an opt-in mode for crash-safe Canal position sync semantics.
• In that mode, skip OnPosSynced for BEGIN QueryEvent so the GTID set is not advanced before the transaction is processed.
• Report XID position sync with force=true, and avoid the final forced OnPosSynced from Canal.Close().
• Add offline unit tests for legacy behavior, transactional behavior, XID force sync, close behavior, and BEGIN detection.

Validation

• go test ./canal -run 'TestIsBeginQuery|TestHandleEventBeginPositionSync|TestHandleEventXIDForcePositionSync|TestClosePositionSync|TestGetShowBinaryLogQuery'
• go test ./canal -run '^$'
• go test ./... -run '^$'
• git diff --check

Full go test ./canal requires a local MySQL instance; without one it fails at 127.0.0.1:3306: connect: connection refused.

[gemini-code-assist]

Code Review

This pull request introduces a TransactionalPosSync configuration option to prevent syncing binlog positions for unfinished transactions. The implementation skips position synchronization on 'BEGIN' query events, forces synchronization on 'XID' events, and omits the final sync during service shutdown when enabled. A new helper function isBeginQuery and comprehensive unit tests were added. Feedback suggests optimizing isBeginQuery by using the bytes package to avoid unnecessary string allocations in a hot path.

[lance6716]

/cc @dveeden

[Copilot]

Pull request overview

Adds an opt-in “transactional position sync” mode to Canal to avoid syncing binlog/GTID positions at transaction start (e.g., BEGIN) and instead force syncing at transaction commit boundaries, improving crash-safety for GTID-based consumers.

Changes:

• Introduce Config.TransactionalPosSync to alter position sync semantics (skip BEGIN, force sync on XID and COMMIT).
• Update Canal.Close() to avoid the final forced OnPosSynced call when transactional mode is enabled.
• Add offline unit tests covering legacy vs transactional behavior, forced sync behavior, and close behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
canal/sync.go	Implements transactional position sync logic (skip BEGIN sync; force sync on XID/COMMIT).
canal/sync_test.go	Adds offline unit tests validating new/legacy position sync semantics.
canal/config.go	Adds TransactionalPosSync config flag with TOML support and docs.
canal/canal.go	Skips forced OnPosSynced on Close() when transactional mode is enabled.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[lance6716]

ptal @dbnski @dveeden

[lance6716]

@serprex I have merged #1153. Can you take a look at this PR? For example, do you think it's good to add a new config item, the default behaviour will not run this logic?

			switch stmt.(type) {
			case *ast.BeginStmt, *ast.SavepointStmt:
				// transaction not yet complete; checkpointing here would skip it on GTID resume
				continue
			}

[serprex]

imo you should drop this PR & take #1153 as the correct way about things

but take my opinion with a grain of salt, peerdb doesn't use canal, so I have no context on its use / design

[lance6716]

imo you should drop this PR & take #1153 as the correct way about things

but take my opinion with a grain of salt, peerdb doesn't use canal, so I have no context on its use / design

We also don't use canal API, so I can't decide if it's worth adding an option to keep wrong but more compatible behaviour.

@refat75 @dbnski @dveeden WDYT

[dbnski]

IMHO #1153 breaks other things. Specifically, as of right now, BEGIN no longer calls OnQueryEvent at all, which I do not think is correct either. QUERY events can carry other information besides just the statement. For example:

#260712 18:10:31 server id 1  end_log_pos 373 CRC32 0xefcd9e47  Query   thread_id=408   exec_time=0     error_code=0
SET TIMESTAMP=1783872631.946989/*!*/;
SET @@session.pseudo_thread_id=408/*!*/;
SET @@session.foreign_key_checks=1, @@session.sql_auto_is_null=0, @@session.unique_checks=1, @@session.autocommit=1/*!*/;
SET @@session.sql_mode=1168113696/*!*/;
SET @@session.auto_increment_increment=1, @@session.auto_increment_offset=1/*!*/;
/*!\C utf8mb4 *//*!*/;
SET @@session.character_set_client=255,@@session.collation_connection=255,@@session.collation_server=255/*!*/;
SET @@session.time_zone='SYSTEM'/*!*/;
SET @@session.lc_time_names=0/*!*/;
SET @@session.collation_database=DEFAULT/*!*/;
/*!80011 SET @@session.default_collation_for_utf8mb4=255*//*!*/;
BEGIN
/*!*/;

All that session meta-data, e.g. the encoding, the variables, are essential for one of my use cases and there is no other way to capture these details. Besides, BEGIN is a legitimate QUERY event and therefore it should be allowed to run the handler.