Skip to content
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

CBG-4549: Refactor resync Init so the reason for a reset is always logged #7410

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

bbrks
Copy link
Member

@bbrks bbrks commented Mar 7, 2025

CBG-4549

  • Refactor resync Init so the reason for a reset is always logged.
    Scenarios covered by subtests in TestResyncDCPInit:
Initialize_new_run_with_empty_cluster_state db:db Resync: Resetting resync process with ID: "..." - no previous run found
Reinitialize_existing_run db:db Resync: Resuming resync with ID: "..."
Reinitialize_completed_run db:db Resync: Resetting resync process with ID: "..." - previous run completed
Force_restart_existing_run db:db Resync: Resetting resync process with ID: "..." - reset option requested
  • Remove outdated resync information (since 3.0 we have cluster-aware resync which won't allow multiple nodes to run resync concurrently)

Integration Tests

Copy link

github-actions bot commented Mar 7, 2025

Redocly previews

base.InfofCtx(ctx, base.KeyAll, "Resync: Attempting to resume resync with resync ID: %s", r.ResyncID)

r.ResyncID = newID.String()
base.InfofCtx(ctx, base.KeyAll, "Resync: Resetting resync process with ID: %q - %s", r.ResyncID, resetReason)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am fine with this change but I wonder if the two common scenarios here actually should not have the log line reset? I am not convinced by this, but I worry reset implies dropping previous bad state.

clusterStatus == nil - technically this is a reset because you are "setting" but I think that this is better stated as just "Starting resync process".

Similarly with statusDoc.State == BackgroundProcessStateCompleted but this case I can see how reset seems like the right word.

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