Save waitTime value for sequence and conductor action (allow parent/child transaction ids) #4819
Conversation
upgle
changed the title
Codecov Report
@@ Coverage Diff @@
## master #4819 +/- ##
=========================================
- Coverage 85.12% 78.52% -6.6%
=========================================
Files 197 197
Lines 8879 8905 +26
Branches 607 610 +3
=========================================
- Hits 7558 6993 -565
- Misses 1321 1912 +591
Continue to review full report at Codecov.
|
|
Awesome! Thank you for adding this. |
upgle
changed the title
|
I've found a performance improvement point for this PR. As you know, using the string builder is much faster than the current version. The blue line shows the performance of the transaction id generator currently used by openwhisk, and the orange line shows the performance of the implementation using the string builder. I'll add a new commit.
(In the chart, the x-axis is the number of repetitions and the y-axis is the time taken.) Source code for performance testingobject TransactionId {
private val dict = ('A' to 'Z') ++ ('a' to 'z') ++ ('0' to '9')
def generateTidByMap(): String = {
(0 until 32).map(_ => dict(ThreadLocalRandom.current().nextInt(dict.size))).mkString("")
}
def generateTidByStringBuilder(): String = {
val sb = new StringBuilder
for (_ <- 1 to 32) {
sb.append(dict(util.Random.nextInt(dict.length)))
}
sb.toString
}
} |
| */ | ||
| @tailrec | ||
| private def findRoot(meta: TransactionMetadata): TransactionMetadata = | ||
| if (meta.parent.isDefined) findRoot(meta.parent.get) else meta |
There was a problem hiding this comment.
@markusthoemmes tailrec work with option map?
| */ | ||
| @tailrec | ||
| private def findRoot(meta: TransactionMetadata): TransactionMetadata = | ||
| if (meta.parent.isDefined) findRoot(meta.parent.get) else meta |
There was a problem hiding this comment.
| if (meta.parent.isDefined) findRoot(meta.parent.get) else meta | |
| meta.parent match { | |
| case Some(parent) => findRoot(parent) | |
| case _ => meta |
There was a problem hiding this comment.
avoid .get even though it's safe, also clearer to me
There was a problem hiding this comment.
@rabbah Thank you for your sample code, I've modified the code.
|
@upgle Sorry for letting this become stale. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4819 +/- ##
==========================================
- Coverage 83.34% 77.33% -6.01%
==========================================
Files 200 201 +1
Lines 9334 9469 +135
Branches 408 394 -14
==========================================
- Hits 7779 7323 -456
- Misses 1555 2146 +591 ☔ View full report in Codecov by Sentry. |
Description
Currently, The openwhisk uses the
waitTimeannotation to store the internal wait time until the action code is provisioned. But this value is only present for non-sequence related activations.Internally invoked actions can't know
waitTimebecause this value is measured based on the start value of the first created TransactionId by topmost action.So those actions invoked by topmost action must have a different TransactionId with a different start value.
To solve this problem I've modified the TransactionId case class to have a parent/child structure.
Define the starting point for internally invoked action
This is the invocation flow of the sequence and conductor action in the controller.
(Each action calls internal actions in a different way, so the starting point is different.)
Sequence action
An
invokeNextActionmethod could be the starting point to create child transactionId.Conductor action
A
tryInvokeNextandinvokeConductormethod could be the starting point to create child transactionId.Creating a new child TransactionId
You can create a new child transaction by calling the childOf method of TransactionId.
Print root transaction id and leaf transaction id together
Related issue and scope
#3083
My changes affect the following components
Types of changes
Checklist: