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

NETOBSERV-617: split big payloads in GRPC exporter #81

Merged
merged 1 commit into from
Jan 4, 2023
Merged

NETOBSERV-617: split big payloads in GRPC exporter #81

merged 1 commit into from
Jan 4, 2023

Conversation

mariomac
Copy link

@mariomac mariomac commented Jan 2, 2023

Only for GRPC exporter, payloads with more than 10000 flows (default configurable value) are split in multiple messages that are submitted sequentially.

entries := make([]*pbflow.Record, 0, len(inputRecords))
for _, record := range inputRecords {
entries = append(entries, flowToPB(record))
}
return &pbflow.Records{
Entries: entries,
var records []*pbflow.Records
Copy link
Member

Choose a reason for hiding this comment

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

Could it be better done in one pass rather than 2 passes?

Copy link
Author

@mariomac mariomac Jan 4, 2023

Choose a reason for hiding this comment

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

I'll do it despite I'm afraid it will look a bit less readable. Also consider that in the second step we aren't copying arrays but just creating references to subslices pointing to the data in the original "entries" slice, so the impact in performance shouldn't be really high (saving the original "entries" reference, which is about 24 bytes, plus the control of the second loop, which will be usually few iterations).
image
But anyway let me do the change and then decide which one do you prefer.

Copy link
Member

Choose a reason for hiding this comment

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

no no, that's fine, you can leave it as it is
I though it involved copying slices

Copy link
Member

Choose a reason for hiding this comment

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

thanks btw for the info!

Copy link
Member

@jotak jotak left a comment

Choose a reason for hiding this comment

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

lgtm, just wondering if a small perf improvement is possible when creating flow slices

@mariomac mariomac merged commit 6a351f2 into netobserv:main Jan 4, 2023
shach33 pushed a commit to shach33/netobserv-ebpf-agent that referenced this pull request Jan 10, 2023
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.

2 participants