tasklog/percentage_task.go: add Complete() method

In commit e90d54dabb45fd61c0c98803e009961800422010 of PR #2329 the
"git/githistory/log" package was added, including the PercentageTask
structure and associated methods, but unlike the WaitingTask which
was added at the same time, no Complete() method was defined for
the PercentageTask structure.

(Note that the "git/githistory/log" package was later renamed to the
"tasklog" package in commits b9ab79ef899c4ee7cb656cdcbed666eed04c3022
and 45c580e60abfa36d0f9affba8b236f0e6516ce1d of PR #2747.)

Other task types such as the ListTask and SimpleTask structures
(introduced in commit 31ffeb94f9e3180939af3019787a5c02b6035851 of
PR #2335 and commit 7a760b6b43d1131bbac89a552cf656b7fa6ce17a of PR #2756,
respectively) provide a Complete() method, and the Meter task type
of the "tq" package, which implemented the Task interface in commit
7c0f9e23f7f806811a7262d3f8ff17ad3b6cbe5b of PR #2732, provides an
equivalent Finish() method.  These methods allow the caller to
explicitly close the channel returned by the Updates() method,
on which the anonymous goroutine started by the Logger.consume() method
for the task is waiting, in a "range" loop on the channel in the
Logger.logTask() method.

One key use of the PercentageTask structure is in the methods of
the Rewriter structure from the "git/githistory" package.  It is
initialized with the number of commits to be rewritten during
a "git lfs migrate" command's traversal of a Git repository's history.
As each commit is rewritten, the Count() method is called to
increment the percentage completed value.  When every commit has
been rewritten, the count reaches the expected total, and the
Count() method closes the channel, thus allowing the task receiving
updates to finish and exit its goroutine.

Under exceptional circumstances, though, the Rewrite() method of
the Rewriter structure may never finish iterating through all
the expected commits, and return in error or via a panic call.
When this happens, the goroutine waiting on the PercentageTask's
updates channel can never exit, leading to a hung process which
never fully exits.

In order to allow the Rewriter's Rewrite() method to define a
deferred function which will always be called when it returns,
under any circumstances, we add a Complete() method for the
PercentageTask structure which sets the number of completed elements
to the expected total in an atomic swap, and then closes the
updates channel if the number of completed elements was previously
less than the expected total.

We also add a test to validate this new method's behaviour, and
update the existing TestPercentageTaskCallsDoneWhenComplete()
function to also confirm that calling the new Complete() method
after the number of completed elements has reached the expected
total does not cause a second attempt to close the updates channel.
This commit is contained in:
Chris Darroch 2023-05-25 23:01:22 -07:00
parent 3d872ab80e
commit 40dc75649b
2 changed files with 40 additions and 0 deletions

@ -78,6 +78,15 @@ func (c *PercentageTask) Entry(update string) {
}
}
// Complete notes that the task is completed by setting the number of
// completed elements to the total number of elements, and if necessary
// closing the Updates channel, which yields the logger to the next Task.
func (c *PercentageTask) Complete() {
if count := atomic.SwapUint64(&c.n, c.total); count < c.total {
close(c.ch)
}
}
// Updates implements Task.Updates and returns a channel which is written to
// when the state of this task changes, and closed when the task is completed.
func (c *PercentageTask) Updates() <-chan *Update {

@ -50,6 +50,37 @@ func TestPercentageTaskCallsDoneWhenComplete(t *testing.T) {
if _, ok := <-task.Updates(); ok {
t.Fatalf("expected channel to be closed")
}
defer func() {
if err := recover(); err != nil {
t.Fatal("tasklog: expected *PercentageTask.Complete() to not panic")
}
}()
task.Complete()
}
func TestPercentageTaskCompleteClosesUpdates(t *testing.T) {
task := NewPercentageTask("example", 10)
select {
case v, ok := <-task.Updates():
if ok {
assert.Equal(t, "example: 0% (0/10)", v.S)
} else {
t.Fatal("expected channel to be open")
}
default:
}
assert.EqualValues(t, 7, task.Count(7))
assert.Equal(t, "example: 70% (7/10)", (<-task.Updates()).S)
task.Complete()
if _, ok := <-task.Updates(); ok {
t.Fatalf("expected channel to be closed")
}
}
func TestPercentageTaskIsThrottled(t *testing.T) {