Skip to content

Conversation

@shajmakh
Copy link
Member

@shajmakh shajmakh commented Oct 6, 2025

Extend the recorder to be configured with 2 more options:

  1. Max size of the NodeRecorder
  2. Coalescing the last PFP record
    see the commits for more context.

@openshift-ci openshift-ci bot requested review from Tal-or and ffromani October 6, 2025 14:11
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 6, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: shajmakh
Once this PR has been reviewed and has the lgtm label, please assign swatisehgal for approval. For more information see the Code Review Process.

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

Details 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

}

func (rs RecordedStatus) Size() int {
byteCount, _ := fmt.Fprint(io.Discard, fmt.Sprintf("%+v\n%+v", rs.Pods, rs.RecordTime))
Copy link
Member

Choose a reason for hiding this comment

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

please remove this

Copy link
Member Author

Choose a reason for hiding this comment

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

do you mean that we should replace it with calculating that ourselves like you originally did? I thought go fmt provides bytesize for free so why not to use it if we are dumping the output to io.Discard

@shajmakh shajmakh force-pushed the add-more-options branch 3 times, most recently from bba0e9a to 41f345f Compare October 7, 2025 10:29
shajmakh and others added 3 commits October 7, 2025 13:32
Support new option that if set to true it would configure the recorders
to push PFP statuses only if they are unique in PFP values compared to
the last existing status, otherwise it just skips pushing the status.
Note that this option works only for the last recorded status and it does
 not affect other statuses in the stack.

Signed-off-by: Shereen Haj <shajmakh@redhat.com>
Place Recorder and NodeRecorder options in one file for easier
navigation.

Signed-off-by: Shereen Haj <shajmakh@redhat.com>
Allow to configure the NodeRecorder with maximum capacity to avoid
extreme growth in recorded PFP statuses size. This is yet another
sealing besides maxCapacity that the node recorder should not cross.
The maxSize takes precesdence over maxCapacity, meaning as long as the
current size of the nodeRecorder does not reach the maximum allowed,
`statuses` slice length is maintained and is allowed to reach the
maximum capacity. Once the new item pushed to the nodeRecorder is
expected to make the recorder reach the max allowed size, `statuses`
will witness reductions in older items to allow the new status to fit,
under the constraint that node recorder will at least have one status no
matter its size. So the boundary is important but the recorder goal
should still be on top.

Signed-off-by: Francesco Romani <fromani@redhat.com>
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