Update abort_old_txes of the consumer group to use locks

[rystsov]

An execution of abort_old_txes could span multiple terms so the so the method could modify new state assuming it's the old state resulting in undefined behavior.

This commit is the rewrite of the reverted f7fc026f in 11474. The problem was caused by:

• do_detach_partition got blocked
• RP ignored blocked do_detach_partition and attempted next op leading double registration of the consumer groups ntp

The op was blocked by a deadlock:

• do_abort_old_txes was waiting for read lock while holding _gate
• do_detach_partition was holding write lock while waiting to the gate to be closed

This version doesn't wait for the read lock to become available and exit do_abort_old_txes releasing the _gate.

It still isn't clear why RP ignored a blocked op

Fixes #11562

Backports Required

• none - not a bug fix
• none - this is a backport
• none - issue does not exist in previous branches
• none - papercut/not impactful enough to backport
• v23.1.x
• v22.3.x
• v22.2.x

Release Notes

• none

[bharathv]

I have a question.. why should shutdown_groups() be within catchup_writelock scope? Does the following break something?

std::vector<group_ptr> groups_for_shutdown;
{
   auto units = co_await p->catchup_lock.hold_write_lock();

    // Becasue shutdown group is async operation we should run it after
    // rehash for groups map
 
    for (auto g_it = _groups.begin(); g_it != _groups.end();) {
        if (g_it->second->partition()->ntp() == p->partition->ntp()) {
            groups_for_shutdown.push_back(g_it->second);
            _groups.erase(g_it++);
            continue;
        }
        ++g_it;
    }
    // if p has background work that won't complete without an abort being
    // requested, then do that now because once the partition is removed from
    // the _partitions container it won't be available in group_manager::stop.
    if (!p->as.abort_requested()) {
        p->as.request_abort();
    }
    _partitions.erase(ntp);
    _partitions.rehash(0);
}
co_await shutdown_groups(std::move(groups_for_shutdown));

[rystsov]

Most likely nothing but it takes effort to reason so I prefer to leave it as is. The PR doesn't introduce this lock there so I think it's safer to left it intact.

[bharathv]

I think this is related to the deadlock.. AFAICT, moving shutdown_groups() out of write scope removes the nested lock/gate dependency and fixes the deadlock, worth exploring.

[rystsov]

Closing the PR in favor of #11621