Skip to content

Synchronizer: Add consolidated sync report and --fail-on-error flag - #258

Merged
dimas-b merged 3 commits into
apache:mainfrom
saidixith002:polaris-synchronizer/sync-report
Jul 27, 2026
Merged

Synchronizer: Add consolidated sync report and --fail-on-error flag#258
dimas-b merged 3 commits into
apache:mainfrom
saidixith002:polaris-synchronizer/sync-report

Conversation

@saidixith002

Copy link
Copy Markdown
Contributor

Motivation

PolarisSynchronizer currently reports progress purely via interleaved SLF4J log lines (clientLogger.info/.error) scattered across each of its 9 sync methods. For any
non-trivial migration — hundreds of catalogs, namespaces, or tables — this is unreadable: there's no way to tell at a glance what succeeded, what failed, or how many entities
of each type were touched without grepping through thousands of log lines.

This is especially painful for automated usage (CI, cron-based backup/sync jobs), where the only way to detect a partial failure today is to scrape logs for ERROR lines.
There's also no way to fail a script/pipeline based on sync outcome — sync-polaris always exits 0, even if individual entities failed to sync.

Additionally, since the sync is designed to be idempotent (only the diff between source and target is applied), a fully-synced re-run currently logs each skipped entity
individually but never surfaces a summary — so a user re-running the tool sees no output at all and can't easily tell "everything is already in sync" from "something silently
didn't run."

What this changes

  • sync-polaris now prints a consolidated synchronization report once, at the end of every run, instead of relying on interleaved logs. It summarizes, per entity type,
    how many entities were created, overwritten, removed, skipped (already in sync), and failed, followed by a list of failures with their identifiers and error
    messages:
    === Synchronization Report ===
    Principal:               3 created, 0 overwritten, 0 removed, 0 skipped, 1 failed
    Catalog:                 2 created, 1 overwritten, 0 removed, 5 skipped, 0 failed
    Table:                  20 created, 0 overwritten, 0 removed, 40 skipped, 2 failed

    Failures:
      - Principal 'alice': <exception message>
      - Table 'db.orders': <exception message>
    ===============================

Entity types with all-zero counts are omitted to keep the report scannable, and the Failures: section is omitted entirely when there are none.

  • Adds a new --fail-on-error flag. By default, sync-polaris still exits 0 regardless of individual entity failures (preserving existing behavior: failures are logged
    and the run continues to completion). With --fail-on-error, the command exits non-zero if the report contains any failures. This is deliberately distinct from the existing
    --halt-on-failure, which stops the run at the first failure — --fail-on-error lets the run finish synchronizing everything it can, then fails afterward, which is
    generally what you want for a CI/cron job (best-effort sync, but still surface that something needs attention).

  • Entities that were already in sync are now tallied under skipped rather than just being logged and forgotten. Without this, an idempotent re-run against an already-synced
    target prints an empty report with no === content in between, which reads as ambiguous ("did it even check anything?") rather than reassuring.

Implementation

  • New EntityType, SyncOutcome, and SynchronizationReport classes (api/.../planning/plan/), mirroring the existing SynchronizationPlan/PlannedAction pattern
    already used elsewhere in this module rather than introducing a new abstraction style.
  • PolarisSynchronizer takes a SynchronizationReport collaborator (constructor injection, same pattern as etagManager) and records an outcome at every
    create/overwrite/remove/skip site across all 9 sync methods.
  • SyncPolarisCommand constructs the report, passes it into PolarisSynchronizer, prints report.render() via the existing console logger after the run completes, and
    returns exit code 1 when --fail-on-error is set and report.hasFailures().
  • README updated with a sample report and documentation for --fail-on-error.

Test plan

  • SynchronizationReportTest: recordSuccess/recordFailure target the correct cells, hasFailures() toggles correctly, render() omits all-zero entity types and the
    empty Failures: section, multiple failures render correctly, and SKIPPED is counted and rendered without being treated as a failure
  • ./gradlew :polaris-synchronizer-api:test :polaris-synchronizer-cli:test passes
  • Manually verified end-to-end against a local docker-compose Polaris source/target setup: first sync shows correct created counts; a second, idempotent run shows
    skipped counts instead of an empty report

Prints a consolidated per-entity-type synchronization report at the end of every run, and adds --fail-on-error to exit non-zero when the report contains failures. Entities that were already in sync (skipped) are now tallied separately in the report so idempotent re-runs don't render an empty report.

@dimas-b dimas-b left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice enhancement 👍 Thanks again for your contribution, @saidixith002 !

Just one minor comment.

throw new IllegalArgumentException("Use recordFailure() to record a failed sync.");
}
Map<SyncOutcome, Integer> outcomeCounts = counts.get(type);
outcomeCounts.put(outcome, outcomeCounts.get(outcome) + 1);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Map.compute()?

@dimas-b

dimas-b commented Jul 23, 2026

Copy link
Copy Markdown

@saidixith002 : please resolve merge conflicts

saidixith002 and others added 2 commits July 27, 2026 11:04
The merge of main into this branch incorrectly combined the
SynchronizationReport and skipIcebergContent constructor parameter
additions from two separately-developed features, leaving a malformed
duplicated parameter list that failed to compile. Merge the two
parameters into a single correct signature and update all call sites.
@saidixith002

Copy link
Copy Markdown
Contributor Author

@dimas-b , Resolved all the conflicts. Please merge

@dimas-b
dimas-b merged commit e5fea02 into apache:main Jul 27, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants