Skip to content

feat(domain multi-tenancy): Store task list name in history tasks#7744

Merged
Shaddoll merged 1 commit intocadence-workflow:masterfrom
Shaddoll:ms
Feb 25, 2026
Merged

feat(domain multi-tenancy): Store task list name in history tasks#7744
Shaddoll merged 1 commit intocadence-workflow:masterfrom
Shaddoll:ms

Conversation

@Shaddoll
Copy link
Member

@Shaddoll Shaddoll commented Feb 24, 2026

What changed?
Store task list name in history tasks which will be used to ensure fair scheduling across task lists in history.

Why?
We store the task list property of history tasks so that we can use that property to ensure fair scheduling of history tasks across task lists. This is a part of project #7724

How did you test it?
unit test
cd ./service/history/execution && go test ./...

Potential risks
N/A

Release notes
N/A

Documentation Changes
N/A


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
Copy link

gitar-bot bot commented Feb 24, 2026

Code Review 👍 Approved with suggestions 0 resolved / 2 findings

Well-structured feature addition that systematically populates task list names across all transfer and timer tasks. Two prior minor findings (comment formatting and missing ephemeral normalization test) remain unresolved but are non-blocking.

💡 Quality: No test coverage for ephemeral task list normalization path

📄 service/history/execution/mutable_state_task_generator.go:700 📄 service/history/execution/mutable_state_task_generator_test.go

The getNormalizedTaskListName function has two branches: ephemeral (returns "__ephemeral__") and non-ephemeral (returns the name as-is). All existing tests use the default TaskListKind (which is TaskListKindNormal = 0), so the ephemeral normalization branch is never exercised.

Since ephemeral normalization is the core novel logic in this PR — it aggregates all ephemeral task lists under a single canonical name to prevent scheduling fragmentation — it would be valuable to have at least one test case that sets TaskListKind: types.TaskListKindEphemeral on the execution info and verifies the task's TaskList field is set to "__ephemeral__" instead of the original name.

A simple unit test for getNormalizedTaskListName itself would also help cover edge cases like empty names with ephemeral kind.

💡 Quality: Comment continuation lines use spaces instead of tabs

📄 common/persistence/tasks.go:44

Lines 44-45 in tasks.go use 4 spaces for indentation on the comment continuation lines, while the rest of the file consistently uses tabs. This will produce inconsistent formatting and may be flagged by gofmt.

L43: 	// GetTaskList returns the name of the task list the task is currently
L44:     // associated with. This may differ from the original task list if the
L45:     // task is a sticky decision task.

Compare with lines 47-49 which correctly use tab indentation for the GetOriginalTaskList comment.

Suggested fix
	// associated with. This may differ from the original task list if the
	// task is a sticky decision task.
Rules ⚠️ 1/2 requirements met

Repository Rules

PR Description Quality Standards: Expand 'Why?' section to explain the prior state problem, how normalized task list names enable fair scheduling mechanisms, and the architectural rationale for storing task lists in task objects
GitHub Issue Linking Requirement: PR links to GitHub issue #7724 in the description

1 rule not applicable. Show all rules by commenting gitar display:verbose.

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 Shaddoll merged commit b3d0695 into cadence-workflow:master Feb 25, 2026
43 checks passed
@Shaddoll Shaddoll deleted the ms branch February 25, 2026 17:35
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