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-1458: reduce loki streams, index FlowLayer #554

Merged
merged 3 commits into from
Feb 2, 2024

Conversation

jotak
Copy link
Member

@jotak jotak commented Feb 1, 2024

No description provided.

@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Feb 1, 2024

@jotak: This pull request references NETOBSERV-1458 which is a valid jira issue.

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

Copy link

openshift-ci bot commented Feb 1, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@jotak jotak added ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. and removed do-not-merge/work-in-progress jira/valid-reference labels Feb 1, 2024
@jotak
Copy link
Member Author

jotak commented Feb 1, 2024

cc @msherif1234 @nathan-weinberg : I would like to compare perf with this set of loki index, against the previous runs and also against this other PR #552

@jotak jotak changed the title NETOBSERV-1458: reduce loki streams, index FlowInfra NETOBSERV-1458: reduce loki streams, index FlowLayer Feb 1, 2024
Copy link

github-actions bot commented Feb 1, 2024

New images:

  • quay.io/netobserv/network-observability-operator:405439d
  • quay.io/netobserv/network-observability-operator-bundle:v0.0.0-405439d
  • quay.io/netobserv/network-observability-operator-catalog:v0.0.0-405439d

They will expire after two weeks.

To deploy this build:

# Direct deployment, from operator repo
IMAGE=quay.io/netobserv/network-observability-operator:405439d make deploy

# Or using operator-sdk
operator-sdk run bundle quay.io/netobserv/network-observability-operator-bundle:v0.0.0-405439d

Or as a Catalog Source:

apiVersion: operators.coreos.com/v1alpha1
kind: CatalogSource
metadata:
  name: netobserv-dev
  namespace: openshift-marketplace
spec:
  sourceType: grpc
  image: quay.io/netobserv/network-observability-operator-catalog:v0.0.0-405439d
  displayName: NetObserv development catalog
  publisher: Me
  updateStrategy:
    registryPoll:
      interval: 1m

@github-actions github-actions bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Feb 1, 2024
@jotak
Copy link
Member Author

jotak commented Feb 1, 2024

Some frontend perf test --
I've been following the same steps that I ran previously and described here: #409 (comment)

---------------------
1st RUN: main
---------------------
streams: ~2K
---------------------
min: 171ms
max: 393ms
avg: ~210ms
=====================
---------------------
2nd RUN: 88ec5d6
Remove some labels
---------------------
streams: ~1.2K
---------------------
min: 1321ms
max: 2784ms
avg: ~1800ms
=====================
---------------------
3rd RUN: 405439d
New indexes
---------------------
streams: ~1.3K
---------------------
min: 202ms
max: 399ms
avg: ~210ms
=====================
---------------------
4th RUN: local build
Only removing Duplicate
---------------------
streams: ~1.3K
---------------------
min: 253ms
max: 493ms
avg: ~300ms
=====================

To summarize: it's clear that the labels introduced in 1.5 caused an increased number of streams (RUN 1). We can remove some labels to reduce that number:

  • Removing both Duplicate and Src/DstType (RUN 2), like it was in 1.4, degrades too much the query performance (x10 slower)
  • Removing only Duplicate (RUN 4) makes us retrieve almost the same number of streams than previously, while keeping pretty good query performance
  • Removing Duplicate and Adding FlowLayer (RUN 3 / this PR) is comparable to RUN 4 for streams but with a slightly better query performance

@jotak jotak marked this pull request as ready for review February 1, 2024 13:04
Copy link

codecov bot commented Feb 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (74bb07b) 57.77% compared to head (b2ff2fe) 57.80%.
Report is 1 commits behind head on release-1.5.

Additional details and impacted files
@@               Coverage Diff               @@
##           release-1.5     #554      +/-   ##
===============================================
+ Coverage        57.77%   57.80%   +0.03%     
===============================================
  Files               72       72              
  Lines             9541     9541              
===============================================
+ Hits              5512     5515       +3     
+ Misses            3690     3688       -2     
+ Partials           339      338       -1     
Flag Coverage Δ
unittests 57.80% <ø> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nathan-weinberg
Copy link
Contributor

@jotak ack will test this! see table in NETOBSERV-1470 for comparisons

Copy link
Contributor

@jpinsonneau jpinsonneau left a comment

Choose a reason for hiding this comment

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

Sounds good to me; let's wait @nathan-weinberg feedback on the perfs before merging

@nathan-weinberg
Copy link
Contributor

/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved QE has approved this pull request label Feb 2, 2024
@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Feb 2, 2024

@jotak: This pull request references NETOBSERV-1458 which is a valid jira issue.

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@jotak
Copy link
Member Author

jotak commented Feb 2, 2024

/approve

Copy link

openshift-ci bot commented Feb 2, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jotak

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved label Feb 2, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit 5450d07 into netobserv:release-1.5 Feb 2, 2024
11 checks passed
@jotak jotak deleted the index-infra branch February 8, 2024 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants