-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Fix build error #5123
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
Fix build error #5123
Conversation
|
@ningyougang thanks ! |
You are welcome. |
| this, | ||
| s"This is the remaining container for ${data.action}. The container will stop after $warmedContainerKeepingTimeout.") | ||
| setTimer(KeepingTimeoutName, Remove, warmedContainerKeepingTimeout) | ||
| startSingleTimer(KeepingTimeoutName, Remove, warmedContainerKeepingTimeout) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
startTimerAtFixedRate instead ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked the source code
def startTimerAtFixedRate(name: String, msg: Any, interval: FiniteDuration): Unit =
startTimer(name, msg, interval, FixedRateMode)
...
/** INTERNAL API */
@InternalApi
private[akka] case object FixedRateMode extends TimerMode {
override def repeat: Boolean = true
}
the method: startTimerAtFixedRate will invoke startTimer with repeat: true,
but for here, it is a one time timer,
So startSingleTimer is ok here.
| } | ||
| unstashAll() | ||
| case _ -> Paused => setTimer(IdleTimeoutName, StateTimeout, idleTimeout) | ||
| case _ -> Paused => startSingleTimer(IdleTimeoutName, StateTimeout, idleTimeout) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
startTimerAtFixedRate instead ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| this, | ||
| s"Remain running activations ${runningActivations.keySet().toString()} when received ClientClosed") | ||
| setTimer(RunningActivationTimeoutName, ClientClosed, runningActivationTimeout) | ||
| startSingleTimer(RunningActivationTimeoutName, ClientClosed, runningActivationTimeout) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wondering if startTimerAtFixedRate should be used instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codecov Report
@@ Coverage Diff @@
## master #5123 +/- ##
==========================================
+ Coverage 28.91% 29.91% +1.00%
==========================================
Files 219 228 +9
Lines 11775 12246 +471
Branches 506 519 +13
==========================================
+ Hits 3405 3664 +259
- Misses 8370 8582 +212
Continue to review full report at Codecov.
|
rabbah
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
307a1b3 to
156c2eb
Compare
Description
Fix build error, and the error is leaded by this pr: Upgrade to Akka 2.6.12: #5065
When i tested, forgot to rebase that pr.
Related issue and scope
My changes affect the following components
Types of changes
Checklist: