tasklog: introduce *SimpleTask
#2756
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
technoweenie
approved these changes
Dec 1, 2017
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
May 26, 2023
The SimpleTask structure of the "tasklog" package, which was introduced in commit 7a760b6 of PR git-lfs#2756, is used at several points in the implementation of the "git lfs fetch" and "git lfs prune" commands, and in each case is constructed with a call to the NewSimpleTask() function, and then passed to a Logger structure's Enqueue() method. However, in commit 0cad488 of PR git-lfs#2767, a Logger.Simple() method was added to both create a new SimpleTask structure and pass it to a Logger structure's Enqueue() method. This Logger.Simple() method was added to be consistent with other existing similar methods such as Logger.List() and Logger.Percentage(). These other methods are used consistently throughout our code to create and enqueue tasks, so we now update the locations where we call NewSimpleTask() to use the Logger.Simple() method instead. This also provides an opportunity to tidy some of the surrounding code and whitespace.
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
May 26, 2023
In commit e90d54d of PR git-lfs#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 b9ab79e and 45c580e of PR git-lfs#2747.) Other task types such as the ListTask and SimpleTask structures (introduced in commit 31ffeb9 of PR git-lfs#2335 and commit 7a760b6 of PR git-lfs#2756, respectively) provide a Complete() method, and the Meter task type of the "tq" package, which implemented the Task interface in commit 7c0f9e2 of PR git-lfs#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.
chrisd8088
added a commit
to chrisd8088/git-lfs
that referenced
this pull request
May 26, 2023
In commit e90d54d of PR git-lfs#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 b9ab79e and 45c580e of PR git-lfs#2747.) Other task types such as the ListTask and SimpleTask structures (introduced in commit 31ffeb9 of PR git-lfs#2335 and commit 7a760b6 of PR git-lfs#2756, respectively) provide a Complete() method, and the Meter task type of the "tq" package, which implemented the Task interface in commit 7c0f9e2 of PR git-lfs#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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request introduces a new
Taskimplementation,*tasklog.SimpleTask, for logging unrelated messages.This functionality is required in replacing the usage of
*progress.Spinnerfrom within thegit-lfs-prune(1)command. Instead of building domain-specific Task implementations, this pull request proposes the introduction of a string-based Task implementation that logs (and formats) strings without formatting guidelines.This allows for quick prototyping of new Logger use-cases without the need to build out domain-specific loggers, and is a good fit for use within the
git-lfs-prune(1)command./cc @git-lfs/core