chore(matching): introduce task list registry

[dkrotx]

What changed?
First step (refactoring) for #7712.
Abstracting the way task-lists are managed by introducing a manager registry.

Why?
Abstracting the way task-lists are managed simplifies matcher: it
doesn't need to care how task-lists managers are stored, retrieved
and deleted and don't need to care about the locks anymore.

In addition, this brings ability to introduce optimized views (like per-domain or
per-shard-name) transparently (preserving the same interface). This will
avoid quadratic complexity in shard-processor' functions which
previously had to iterate over the entire list of task list managers for every single shard.
This should greatly impact performance in production where we have
thousands of task-lists in a single matching instance.

Potential risks
None, this should be a transparent optimisation

Release notes

Documentation Changes

---

Reviewer Validation

PR Description Quality (check these before reviewing code):

• "What changed" provides a clear 1-2 line summary
  • Project Issue is linked
• "Why" explains the full motivation with sufficient context
• Testing is documented:
  • Unit test commands are included (with exact go test invocation)
  • Integration test setup/commands included (if integration tests were run)
  • Canary testing details included (if canary was mentioned)
• Potential risks section is thoughtfully filled out (or legitimately N/A)
• Release notes included if this completes a user-facing feature
• Documentation needs are addressed (or noted if uncertain)

[gitar-bot]

Code Review ✅ Approved 3 resolved / 3 findings

Clean refactoring that properly encapsulates task list management into a well-designed ManagerRegistry. All three previously identified issues (redundant name check, off-by-one in String(), nil mockRegistry in test helper) have been resolved in the current code.

✅ 3 resolved

✅ Bug: Test helper uses nil mockRegistry instead of local variable

📄 service/matching/tasklist/shard_processor_test.go:41
In newShardProcessorTestData, the ShardProcessorParams at line 41 uses td.mockRegistry which is nil (zero-value from var td shardProcessorTestData at line 32). The actual mock is in the local variable mockRegistry (created at line 36) but it's never assigned to td.mockRegistry before being used in params.

This means NewShardProcessor(params) receives TaskListsRegistry: nil, which will fail the validateSPParams check (params.TaskListsRegistry == nil → error). Every test calling newShardProcessorTestData will fail with require.NoError(t, err) at line 47.

The fix is to assign mockRegistry to td.mockRegistry before constructing params, or use mockRegistry directly in the params.

✅ Quality: Redundant name check after ManagersByTaskListName filter

📄 service/matching/tasklist/shard_processor.go:97
getShardLoad calls sp.taskListsRegistry.ManagersByTaskListName(sp.shardID) which already filters managers by name matching sp.shardID. Then the loop body at line 97 checks if tlMgr.TaskListID().name == sp.shardID again, which is always true for every element in the returned slice. This redundant check should be removed for clarity.

✅ Edge Case: Off-by-one: String() now prints 1001 items instead of 1000

📄 service/matching/handler/engine.go:248
The old code called getTaskLists(1000) which returned at most 1000 managers. The new code breaks when i >= 1000, but since i is 0-indexed, this prints items at indices 0 through 1000 (1001 items total). This is a minor behavioral change. If the intent is to cap at 1000, the condition should be i >= 999.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[Shaddoll]

Can we move the task list creation logic into the registry so that the synchronization happens in that component? We can remove this lock then.

[dkrotx]

I thought about this originally, but task creation / initialisation is quite big by itself and has a lot of dependencies on engine.
I think this lock is OK since we need to guard task-creation anyway. I can do a small refactoring afterwards to have this lock in defer, though, so it's only mentioned once with a very clear scope. But currently I kept this exactly as the lock (for taskLists) we had previously.

[Shaddoll]

that's fair