Skip to content

Commit 93907ca

Browse files
authored
Remove tasks_expired metric, superseded by tasks_dropped (#10642)
## What changed? Removes the per-task-queue **`tasks_expired`** counter (`ExpiredTasksPerTaskQueueCounter`) and everything that existed only for it: its tags (`TaskExpireStage*`, `TaskInvalidTag`, the `taskExpireStage` key), the `getInvalidTaskTag` helper, all six emission sites in matching, and the `TestGetInvalidTaskTag` test. Surrounding expiry/validation logic and the `tasks_dropped` emissions are untouched. ## Why? `tasks_expired` is fully superseded by `tasks_dropped` (#10468), which reports the same events with finer `reason` tags (`expired_read`, `expired_memory`, `invalid`). This is the agreed post-release cleanup. ## How did you test it? - [x] built (`make bins` / `go build ./...`) - [x] run locally and tested manually - [x] covered by existing tests - [ ] added new unit test(s) - [ ] added new functional test(s) Signed-off-by: Sandeep Balaji <sandeep.balaji@temporal.io>
1 parent f3d7f01 commit 93907ca

9 files changed

Lines changed: 0 additions & 38 deletions

File tree

common/metrics/metric_defs.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1240,7 +1240,6 @@ var (
12401240
)
12411241
SyncThrottlePerTaskQueueCounter = NewCounterDef("sync_throttle_count")
12421242
BufferThrottlePerTaskQueueCounter = NewCounterDef("buffer_throttle_count")
1243-
ExpiredTasksPerTaskQueueCounter = NewCounterDef("tasks_expired") // TODO: remove tasks_expired since it is superseded by tasks_dropped (expired_read / expired_memory reasons).
12441243
ForwardedPerTaskQueueCounter = NewCounterDef("forwarded_per_tl")
12451244
PriorityBacklogForwardedPerTaskQueueCounter = NewCounterDef("priority_backlog_forwarded")
12461245
ForwardTaskErrorsPerTaskQueue = NewCounterDef("forward_task_errors")

common/metrics/tags.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ const (
4747
// See server.api.enums.v1.ReplicationTaskType
4848
replicationTaskType = "replicationTaskType"
4949
replicationTaskPriority = "replicationTaskPriority"
50-
taskExpireStage = "task_expire_stage"
5150
taskAddResult = "task_add_result"
5251
versioningBehavior = "versioning_behavior"
5352
continueAsNewVersioningBehavior = "continue_as_new_versioning_behavior"
@@ -587,10 +586,6 @@ func ToUnversionedTag(version string) Tag {
587586
return Tag{Key: toUnversioned, Value: falseValue}
588587
}
589588

590-
var TaskExpireStageReadTag = Tag{Key: taskExpireStage, Value: "read"}
591-
var TaskExpireStageMemoryTag = Tag{Key: taskExpireStage, Value: "memory"}
592-
var TaskInvalidTag = Tag{Key: taskExpireStage, Value: "invalid"}
593-
594589
// ClientNameTag returns a new client_name tag for the SDK client name.
595590
func ClientNameTag(value string) Tag {
596591
if len(value) == 0 {

service/matching/fair_task_reader.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -513,7 +513,6 @@ func (tr *fairTaskReader) mergeTasksLocked(tasks []*persistencespb.AllocatedTask
513513
// Expired tasks are added as pre-acked (nil) so they participate in
514514
// readLevel calculation above and advance ackLevel + get GC'd below.
515515
tr.outstandingTasks.Put(level, nil)
516-
metrics.ExpiredTasksPerTaskQueueCounter.With(tr.backlogMgr.metricsHandler).Record(1, metrics.TaskExpireStageReadTag)
517516
recordDroppedTask(tr.backlogMgr.metricsHandler, dropReasonExpiredRead)
518517
continue
519518
}

service/matching/metrics_util.go

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,15 +36,6 @@ func (r dropReason) tag() metrics.Tag {
3636
}
3737
}
3838

39-
// getInvalidTaskTag returns the tasks_expired stage tag for a task being dropped
40-
// due to in-memory expiry or validation failure.
41-
func getInvalidTaskTag(task *internalTask) metrics.Tag {
42-
if IsTaskExpired(task.event.AllocatedTaskInfo) {
43-
return metrics.TaskExpireStageMemoryTag
44-
}
45-
return metrics.TaskInvalidTag
46-
}
47-
4839
// getDroppedTaskExpiryReason returns the drop reason for a task being dropped due to
4940
// in-memory expiry or validation failure.
5041
func getDroppedTaskExpiryReason(task *internalTask) dropReason {

service/matching/metrics_util_test.go

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,18 +22,6 @@ func backlogTaskWithExpiry(t *testing.T, expiry *timestamppb.Timestamp) *interna
2222
}, func(*internalTask, taskResponse) {})
2323
}
2424

25-
func TestGetInvalidTaskTag(t *testing.T) {
26-
t.Run("expired -> memory stage", func(t *testing.T) {
27-
task := backlogTaskWithExpiry(t, timestamppb.New(time.Now().Add(-time.Minute)))
28-
require.Equal(t, metrics.TaskExpireStageMemoryTag, getInvalidTaskTag(task))
29-
})
30-
31-
t.Run("not expired -> invalid", func(t *testing.T) {
32-
task := backlogTaskWithExpiry(t, nil)
33-
require.Equal(t, metrics.TaskInvalidTag, getInvalidTaskTag(task))
34-
})
35-
}
36-
3725
func TestGetDroppedTaskExpiryReason(t *testing.T) {
3826
t.Run("expired -> expired_memory", func(t *testing.T) {
3927
task := backlogTaskWithExpiry(t, timestamppb.New(time.Now().Add(-time.Minute)))

service/matching/physical_task_queue_manager.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -525,7 +525,6 @@ func (c *physicalTaskQueueManagerImpl) PollTask(
525525
// history, but this is more efficient.
526526
if task.event != nil && IsTaskExpired(task.event.AllocatedTaskInfo) {
527527
// task is expired while polling
528-
c.metricsHandler.Counter(metrics.ExpiredTasksPerTaskQueueCounter.Name()).Record(1, metrics.TaskExpireStageMemoryTag)
529528
task.finish(taskFinishResult{dropReason: dropReasonExpiredMemory})
530529
continue
531530
}
@@ -580,8 +579,6 @@ func (c *physicalTaskQueueManagerImpl) ProcessSpooledTask(
580579
task *internalTask,
581580
) error {
582581
if !c.taskValidator.maybeValidate(task.event.AllocatedTaskInfo, c.queue.TaskType()) {
583-
var invalidTaskTag = getInvalidTaskTag(task)
584-
c.metricsHandler.Counter(metrics.ExpiredTasksPerTaskQueueCounter.Name()).Record(1, invalidTaskTag)
585582
task.finish(taskFinishResult{dropReason: getDroppedTaskExpiryReason(task)})
586583
// Don't try to set read level here because it may have been advanced already.
587584

service/matching/pri_matcher.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -210,10 +210,7 @@ func (tm *priTaskMatcher) forwardTask(task *internalTask) (bool, error) {
210210
// to the head of the backlog, which is what taskValidator expects.
211211
maybeValid := tm.validator.maybeValidate(task.event.AllocatedTaskInfo, tm.fwdr.partition.TaskType())
212212
if !maybeValid {
213-
var invalidTaskTag = getInvalidTaskTag(task)
214-
215213
// consider this task expired while processing.
216-
tm.metricsHandler.Counter(metrics.ExpiredTasksPerTaskQueueCounter.Name()).Record(1, invalidTaskTag)
217214
task.finish(taskFinishResult{dropReason: getDroppedTaskExpiryReason(task)})
218215

219216
// Stay alive as long as we're invalidating tasks
@@ -269,8 +266,6 @@ func (tm *priTaskMatcher) validateTasksOnRoot(retrier backoff.Retrier) {
269266
maybeValid := tm.validator == nil || tm.validator.maybeValidate(task.event.AllocatedTaskInfo, tm.partition.TaskType())
270267
if !maybeValid {
271268
// We found an invalid one, complete it and go back for another immediately.
272-
var invalidStageTag = getInvalidTaskTag(task)
273-
tm.metricsHandler.Counter(metrics.ExpiredTasksPerTaskQueueCounter.Name()).Record(1, invalidStageTag)
274269
task.finish(taskFinishResult{dropReason: getDroppedTaskExpiryReason(task)})
275270

276271
// Stay alive as long as we're invalidating tasks

service/matching/pri_task_reader.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,6 @@ func (tr *priTaskReader) processTaskBatch(tasks []*persistencespb.AllocatedTaskI
250250

251251
if IsTaskExpired(t) {
252252
// task expired when we read it
253-
metrics.ExpiredTasksPerTaskQueueCounter.With(tr.backlogMgr.metricsHandler).Record(1, metrics.TaskExpireStageReadTag)
254253
recordDroppedTask(tr.backlogMgr.metricsHandler, dropReasonExpiredRead)
255254
return true
256255
}

service/matching/task_reader.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,6 @@ func (tr *taskReader) addTasksToBuffer(
253253
for _, t := range tasks {
254254
if IsTaskExpired(t) {
255255
// task is expired when "add tasks to buffer" is called, so when we read it
256-
metrics.ExpiredTasksPerTaskQueueCounter.With(tr.taggedMetricsHandler()).Record(1, metrics.TaskExpireStageReadTag)
257256
recordDroppedTask(tr.taggedMetricsHandler(), dropReasonExpiredRead)
258257
// Also increment readLevel for expired tasks otherwise it could result in
259258
// looping over the same tasks if all tasks read in the batch are expired

0 commit comments

Comments
 (0)