Skip to content

CreateGroup compensating delete is non-atomic → orphan group rows (needs async reconcile) #394

Description

@an9xyz

Summary

group.CreateGroup commits the group + members in a DB transaction, then creates the IM channel after tx.Commit(). If IMCreateOrUpdateChannel fails, it runs a best-effort compensating delete of the already-committed group / group_member rows. That compensating delete is itself non-atomic and only logs on failure — so if it also fails (e.g. DB blip right after the IM error), the group row survives orphaned: a group with no IM channel that clients can never message.

This is pre-existing, shared-layer behavior (all create paths — Web / Bot / integration — go through group.CreateGroup), not introduced by any recent PR. It was the root-cause backdrop for the idempotency P1 in #377 and was explicitly deferred from the #381 follow-up umbrella as the one cross-cutting data-consistency item warranting its own issue.

Split out of #381 (PR #383 closed the cheap/local follow-ups; the remaining advisory items were accepted as trade-offs).

Where

modules/group/service.go (around L1230–L1294):

// 提交事务
if err := tx.Commit(); err != nil { ... }       // group + group_member committed here

...

// 创建 IM 频道
err = s.ctx.IMCreateOrUpdateChannel(&config.ChannelCreateReq{...})
if err != nil {
    s.Error("create IM channel failed, performing compensating rollback", ...)
    // Compensating delete runs OUTSIDE any transaction (tx already committed),
    // and each failure is only logged — not retried, not durably queued.
    if _, delErr := s.ctx.DB().DeleteFrom("group_member").Where("group_no=?", groupNo).Exec(); delErr != nil {
        s.Error("compensating delete group_member failed", ...)   // <- orphan window
    }
    if _, delErr := s.ctx.DB().DeleteFrom("group").Where("group_no=?", groupNo).Exec(); delErr != nil {
        s.Error("compensating delete group failed", ...)          // <- orphan window
    }
    return nil, errors.New("failed to create IM channel, group has been rolled back")
}

Failure modes producing an orphan

  • IM channel create fails and the subsequent compensating DELETE fails (DB unavailable, deadlock, connection drop).
  • Process crash between tx.Commit() and a successful IM-channel create / compensating delete.

In both cases a group row (and possibly group_member rows) remains with no backing IM channel.

Why it can't be a point fix

The IM channel lives in WuKongIM, not the SQL transaction, so it can't be enrolled in tx. The local compensating delete narrows but cannot eliminate the window — any best-effort cleanup that only logs on failure leaves a residue. Closing it durably needs an async reconcile path, e.g.:

  • a periodic/queued job that finds group rows with no corresponding IM channel (or a channel_synced flag set only after a confirmed IM create) and either re-creates the channel or hard-deletes the orphan, with retry + backoff;
  • and/or an outbox-style record so the IM-create + cleanup intent survives a process crash.

Acceptance criteria

  • A group can never be left queryable/listable without a backing IM channel after a create failure (eventually consistent is acceptable; the orphan must be reconciled, not permanent).
  • Reconcile logic is observable (metric/log for orphans detected & resolved) and idempotent.
  • Behavior verified for both failure modes above (IM-fail + delete-fail, and crash-between-commit-and-IM).

Notes

  • Severity: low-frequency but real data-consistency hole; not exploitable, not a regression.
  • Scope: shared group.CreateGroup — fix benefits Web / Bot / integration alike.

Refs #381, #377.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions