Disable tablet migration on repair

Michal-Leszczynski commented 3 months ago

Fixes #3773

karol-kokoszka commented 3 months ago

@Michal-Leszczynski I know I overlooked in some previous review, but this part (not introduced in this PR directly)

// getTabletKs returns set of tablet replicated keyspaces.
func getTabletKs(ctx context.Context, client *Client) *strset.Set {
    out := strset.New()
    // Assume that errors indicate that endpoints rejected 'replication' param,
    // which means that given Scylla version does not support tablet API.
    // Other errors will be handled on other API calls.
    tablets, err := client.ReplicationKeyspaces(ctx, ReplicationTablet)
    if err != nil {
        return out
    }
    vnodes, err := client.ReplicationKeyspaces(ctx, ReplicationVnode)
    if err != nil {
        return out
    }
    // Even when both API calls succeeded, we need to validate
    // that the 'replication' param wasn't silently ignored.
    out.Add(tablets...)
    if out.HasAny(vnodes...) {
        return strset.New()
    }
    return out
}

Is problematic...

Errors are hidden. Can you log these errors ?

Michal-Leszczynski commented 2 months ago

@karol-kokoszka the logging comment was addressed. Could you take a look?

Michal-Leszczynski commented 2 months ago

@karol-kokoszka could you take a look?

scylladb / scylla-manager

Disable tablet migration on repair #3785