Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate liveness of task list into a dedicated entity #5105

Merged
merged 1 commit into from
Feb 21, 2023

Conversation

Shaddoll
Copy link
Contributor

What changed?
pick temporalio/temporal#1699

  • Mark liveness when adding new tasks from history service
  • Mark liveness when polling tasks from frontend service
  • Unit test

Why?
Improve readability

How did you test it?
New unit tests

Potential risks

Release notes

Documentation Changes

@Shaddoll Shaddoll force-pushed the refactor-matching branch 2 times, most recently from 6276998 to f95a87f Compare February 17, 2023 20:22
@Shaddoll Shaddoll changed the title Refactor matching Separate liveness of task queue into a dedicated entity Feb 17, 2023
@Shaddoll Shaddoll changed the title Separate liveness of task queue into a dedicated entity Separate liveness of task list into a dedicated entity Feb 17, 2023
@Shaddoll Shaddoll force-pushed the refactor-matching branch 2 times, most recently from 62f02bd to 3cf0e06 Compare February 18, 2023 00:44
@coveralls
Copy link

coveralls commented Feb 18, 2023

Pull Request Test Coverage Report for Build 0186750b-9e81-48eb-a5b9-519dab7da2a2

  • 80 of 84 (95.24%) changed or added relevant lines in 4 files are covered.
  • 83 unchanged lines in 15 files lost coverage.
  • Overall coverage decreased (-0.02%) to 57.18%

Changes Missing Coverage Covered Lines Changed/Added Lines %
common/clock/time_source.go 0 2 0.0%
service/matching/liveness.go 52 54 96.3%
Files with Coverage Reduction New Missed Lines %
common/types/shared.go 1 41.76%
service/history/queue/timer_queue_processor_base.go 1 77.26%
common/task/weightedRoundRobinTaskScheduler.go 2 89.12%
common/util.go 2 52.31%
service/history/execution/mutable_state_builder.go 2 68.82%
service/history/task/transfer_active_task_executor.go 2 72.15%
service/matching/matcher.go 2 91.46%
common/persistence/nosql/nosqlplugin/cassandra/workflow.go 3 59.55%
common/persistence/statsComputer.go 3 93.57%
common/task/fifoTaskScheduler.go 3 84.54%
Totals Coverage Status
Change from base Build 01866e5e-f4b8-4b21-8878-657f530d3fce: -0.02%
Covered Lines: 85049
Relevant Lines: 148739

💛 - Coveralls

@@ -230,6 +237,10 @@ func (c *taskListManagerImpl) handleErr(err error) error {
// be written to database and later asynchronously matched with a poller
func (c *taskListManagerImpl) AddTask(ctx context.Context, params addTaskParams) (bool, error) {
c.startWG.Wait()
if params.forwardedFrom == "" {
// request sent by history service
c.liveness.markAlive(time.Now())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't need to be here in this diff, but it'd be nice to pull this time-source from the taskListManagerImpl in future for testability

service/matching/taskListManager_test.go Outdated Show resolved Hide resolved
service/matching/taskListManager_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@davidporter-id-au davidporter-id-au left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!

@Shaddoll Shaddoll merged commit f9d04ea into uber:master Feb 21, 2023
@Shaddoll Shaddoll deleted the refactor-matching branch February 21, 2023 18:23
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.

3 participants