Skip to content

Insert perf metrics into operator #315

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Conversation

xuanying2020
Copy link
Contributor

What changes were proposed in this pull request?

Insert perf metrics into operator.

Why are the changes needed?

Profile the operator performance.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

No.

Comment on lines +75 to +86
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<version>4.13.2</version>
<scope>test</scope>
<exclusions>
<exclusion>
<groupId>org.junit.vintage</groupId>
<artifactId>junit-vintage-engine</artifactId>
</exclusion>
</exclusions>
</dependency>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have junit, use it from parent pom.

Comment on lines +77 to +83
var rb: RowBatch = null
if (allGroupedData.nonEmpty) {
rb = RowBatch(allGroupedData.next())
} else {
rb = RowBatch(Seq.empty)
}
rb
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we fixed it in the previous pr. Why is it here again?

}

override def closeImpl(): Unit = {}

override def outputSchema(): Seq[(String, LynxType)] = schema

override def getOperatorName(): String = "Aggregation"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is not needed. we can get operator name in ExecutionOperator

@@ -117,6 +117,8 @@ case class CreateOperator(

override def outputSchema(): Seq[(String, LynxType)] = schema

override def getOperatorName(): String = "Create"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question as above

}
// to be implemented by concrete operators
def getNextImpl(): RowBatch

def getOperatorName(): String
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can get different operator names by using: getClass.getName. It will automatically determine at runtime.

@LianxinGao
Copy link
Contributor

remove all the method override def getOperatorName(): String

Copy link
Contributor

@terrytangyuan terrytangyuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need a rebase

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants