refactor: extract apply_work inner-loop logic into helper f…

[rasifr]

…unctions

Break the monolithic apply_work() loop into focused helpers: apply_init_contexts, apply_maybe_reload_config, apply_wait_events, apply_maybe_cleanup_resolutions, apply_check_readonly (ReadonlyAction enum), apply_recv_from_stream, apply_dispatch_message, apply_end_of_iteration, apply_exception_reset. No behaviour change.

[coderabbitai]

📝 Walkthrough

Walkthrough

The refactoring introduces a set of modularized static helper functions to decompose apply_work() into manageable pieces: centralized context initialization, safe event consumption from libpq, read-only mode handling, stream message receipt and dispatch, standardized end-of-iteration cleanup, and consolidated exception reset with replay triggering. The main loop is rewritten to invoke these helpers in sequence, and connection-class error detection now uses a dedicated predicate.

Changes

Apply Worker Control Flow Refactoring

Layer / File(s)	Summary
Helper function declarations src/spock_apply.c	Forward declarations for context initialization, event waiting, read-only branching, stream receive/dispatch, iteration cleanup, exception reset, and connection-error classification helpers.
Helper function implementations src/spock_apply.c	Complete implementations of all helper functions: context creation, safe libpq event consumption with connection/timing/liveness error handling, read-only mode logic, stream message receive with keepalive feedback, end-of-iteration feedback and sync-table processing, first-exception reset with replay triggering, and connection-error classification by error code and status.
Apply work refactored loop and error handling src/spock_apply.c	Updated apply_work() startup calls apply_init_contexts() and sets idle activity. Main loop now invokes helpers sequentially: wait for events, check read-only behavior, read from stream or replay queue, dispatch messages, and perform end-of-iteration work. Exception handler classifies connection-class errors via predicate for immediate rethrow; non-connection errors delegate first-exception reset to helper with conditional replay.

Poem

A rabbit refactored with care,
Breaking loops into helpers so fair,
Each function now small, composable, clean,
The clearest apply-work ever seen! 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 76.92% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is partially related to the changeset. It mentions extracting helper functions from apply_work inner-loop logic, which matches the core refactoring, but is truncated and doesn't fully convey the scope of changes.
Description check	✅ Passed	The description is related to the changeset. It lists the helper functions extracted and states 'No behaviour change', which aligns with the refactoring summary of modularizing apply_work through new static helper functions.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch refactor/SPOC-XXX/extract-apply-work-helpers

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 duplication

Metric	Results
Duplication	0

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[coderabbitai]

🧹 Nitpick comments (1)

src/spock_apply.c (1)

3370-3383: 💤 Low value

Consider adding a defensive check for my_exception_log_index >= 0.

The code accesses exception_log_ptr[my_exception_log_index] after checking that exception_log_ptr != NULL, but doesn't verify that my_exception_log_index >= 0. While connection-class errors before the first BEGIN would be rethrown via is_connection_class_error(), a non-connection error during early startup (before handle_begin allocates a slot) could reach this code with my_exception_log_index == -1.

This is pre-existing behavior (the logic was moved from inline code), but adding a defensive check would make it safer.

🛡️ Suggested defensive fix

 	/*
 	 * Save the initial exception message and operation type so we can
 	 * include them in the exception_log if operations succeed on retry.
 	 */
-	if (exception_log_ptr != NULL)
+	if (exception_log_ptr != NULL && my_exception_log_index >= 0)
 	{
 		snprintf(exception_log_ptr[my_exception_log_index].initial_error_message,
 				 sizeof(exception_log_ptr[my_exception_log_index].initial_error_message),

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/spock_apply.c` around lines 3370 - 3383, Add a defensive check that
my_exception_log_index is non-negative before indexing exception_log_ptr: after
confirming exception_log_ptr != NULL, verify my_exception_log_index >= 0 and
only then assign to
exception_log_ptr[my_exception_log_index].initial_error_message and
.failed_action using edata->message and xact_action_counter; if
my_exception_log_index is negative, skip these assignments (or handle via the
existing is_connection_class_error()/handle_begin logic) to avoid out-of-bounds
access. Ensure references to exception_log_ptr, my_exception_log_index,
initial_error_message, failed_action, edata, xact_action_counter,
is_connection_class_error(), and handle_begin are used to locate and implement
the guard.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/spock_apply.c`:
- Around line 3370-3383: Add a defensive check that my_exception_log_index is
non-negative before indexing exception_log_ptr: after confirming
exception_log_ptr != NULL, verify my_exception_log_index >= 0 and only then
assign to exception_log_ptr[my_exception_log_index].initial_error_message and
.failed_action using edata->message and xact_action_counter; if
my_exception_log_index is negative, skip these assignments (or handle via the
existing is_connection_class_error()/handle_begin logic) to avoid out-of-bounds
access. Ensure references to exception_log_ptr, my_exception_log_index,
initial_error_message, failed_action, edata, xact_action_counter,
is_connection_class_error(), and handle_begin are used to locate and implement
the guard.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 808efc3f-6323-4f6c-9b9d-fabe69d8532c

📥 Commits

Reviewing files that changed from the base of the PR and between 3886058 and 79134c2.

📒 Files selected for processing (1)

• src/spock_apply.c