Skip to content
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

addiing parallelism to speed up performace issue #6111 #6139

Closed

Conversation

Insomniac2904
Copy link

@Insomniac2904 Insomniac2904 commented Oct 29, 2024

Which problem is this PR solving?

Description of the changes

  • Added t.parallel() in test functions and used goroutines for checking occurrences in LogMatcher function.

How was this change tested?

  • GOMAXPROCS=1 go test -parallel 128 -p 16 -json ./... | go run github.com/roblaszczak/vgt@latest

Checklist

Results

@Insomniac2904 Insomniac2904 requested a review from a team as a code owner October 29, 2024 09:23
@dosubot dosubot bot added the performance label Oct 29, 2024
Insomniac2904 and others added 3 commits October 29, 2024 15:04
… logger.go LogMatcher function

Signed-off-by: Insomniac2904 <[email protected]>
fix function name in comment

Signed-off-by: cangqiaoyuzhuo <[email protected]>
Signed-off-by: Insomniac2904 <[email protected]>
## Which problem is this PR solving?
The `component.UseLocalHostAsDefaultHost` gate has been removed from
otel collector since v0.112.0. The example failed to start `jaeger`
because of the invalid argument.

## Description of the changes
- Remove the invalid command line argument.
- Set endpoints for `otlpreceiver` explicitly.

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [ ] I have added unit tests for the new functionality
- [ ] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

Signed-off-by: haoqixu <[email protected]>
Co-authored-by: Yuri Shkuro <[email protected]>
Signed-off-by: Insomniac2904 <[email protected]>
var LogMatcher = func(occurrences int, subStr string, logs []string) (bool, string) {
errMsg := fmt.Sprintf("subStr '%s' does not occur %d time(s) in %v", subStr, occurrences, logs)
if len(logs) < occurrences {
return false, errMsg
}

// Count occurrences in parallel
Copy link
Member

Choose a reason for hiding this comment

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

why?

Copy link
Author

Choose a reason for hiding this comment

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

Initially I thought that as log volume increases , concurrent checking for the occurrences would help. But now that I checked the function calls I see that none of the calls affect the testing time of the command that was mentioned in the issue. I have removed the changes to my code locally . Should I submit a new PR? Are there any more changes that I need to do ?
Thanks.

@yurishkuro yurishkuro closed this Nov 1, 2024
@yurishkuro
Copy link
Member

Please read the ticket description carefully.

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

Successfully merging this pull request may close these issues.

Improve unit test speed
4 participants