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

OCPBUGS-17157: improve watching semantics around CSVs #3003

Conversation

stevekuznetsov
Copy link
Member

operators/olm: use a partial object metadata watch for copied CSVs

All we ever ned to know about copied CSVs is their metadata. No need to
prune objects in memory, it's better to never allocate the memory to
deserilize them in the first place.

Signed-off-by: Steve Kuznetsov skuznets@redhat.com


operators/catalog: don't watch copied CSVs

As far as I can tell, we never do anything with them.

Signed-off-by: Steve Kuznetsov skuznets@redhat.com


@openshift-ci-robot openshift-ci-robot added jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. labels Aug 2, 2023
@openshift-ci-robot
Copy link
Collaborator

@stevekuznetsov: This pull request references Jira Issue OCPBUGS-17157, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.14.0) matches configured target version for branch (4.14.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @jianzhangbjz

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

operators/olm: use a partial object metadata watch for copied CSVs

All we ever ned to know about copied CSVs is their metadata. No need to
prune objects in memory, it's better to never allocate the memory to
deserilize them in the first place.

Signed-off-by: Steve Kuznetsov skuznets@redhat.com


operators/catalog: don't watch copied CSVs

As far as I can tell, we never do anything with them.

Signed-off-by: Steve Kuznetsov skuznets@redhat.com


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 kubernetes/test-infra repository.

@stevekuznetsov
Copy link
Member Author

Depends on operator-framework/api#289

go.mod Outdated Show resolved Hide resolved
@stevekuznetsov stevekuznetsov force-pushed the skuznets/catalog-operator-csv branch 3 times, most recently from 8d1fb74 to 9c2ec9c Compare August 2, 2023 23:56
@stevekuznetsov stevekuznetsov force-pushed the skuznets/catalog-operator-csv branch 3 times, most recently from e7821f3 to ef9f9a3 Compare August 3, 2023 14:59
Copy link
Contributor

@perdasilva perdasilva left a comment

Choose a reason for hiding this comment

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

7f5

Nice one! Thank you ^^
We have a downstream only plug-in that creates a copiedcsv informer - do you think we'd benefit with this change there as well?

@openshift-ci
Copy link

openshift-ci bot commented Aug 3, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: perdasilva, stevekuznetsov

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 Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 3, 2023
@stevekuznetsov
Copy link
Member Author

@perdasilva interesting! Yeah, I'll get that when I downstream this, thanks for the call-out.

As far as I can tell, we never do anything with them.

Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
All we ever ned to know about copied CSVs is their metadata. No need to
prune objects in memory, it's better to never allocate the memory to
deserilize them in the first place.

Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
@perdasilva
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 4, 2023
@perdasilva perdasilva merged commit 2c3434a into operator-framework:master Aug 4, 2023
16 checks passed
@openshift-ci-robot
Copy link
Collaborator

@stevekuznetsov: Jira Issue OCPBUGS-17157: Some pull requests linked via external trackers have merged:

The following pull requests linked via external trackers have not merged:

These pull request must merge or be unlinked from the Jira bug in order for it to move to the next state. Once unlinked, request a bug refresh with /jira refresh.

Jira Issue OCPBUGS-17157 has not been moved to the MODIFIED state.

In response to this:

operators/olm: use a partial object metadata watch for copied CSVs

All we ever ned to know about copied CSVs is their metadata. No need to
prune objects in memory, it's better to never allocate the memory to
deserilize them in the first place.

Signed-off-by: Steve Kuznetsov skuznets@redhat.com


operators/catalog: don't watch copied CSVs

As far as I can tell, we never do anything with them.

Signed-off-by: Steve Kuznetsov skuznets@redhat.com


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 kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants