Skip to content

Added TopologySpreadConstraints to the backingstore pod for better performance#1401

Merged
aayushchouhan09 merged 2 commits intonoobaa:masterfrom
aayushchouhan09:topology-const
Aug 5, 2024
Merged

Added TopologySpreadConstraints to the backingstore pod for better performance#1401
aayushchouhan09 merged 2 commits intonoobaa:masterfrom
aayushchouhan09:topology-const

Conversation

@aayushchouhan09
Copy link
Member

@aayushchouhan09 aayushchouhan09 commented Jul 31, 2024

Explain the changes

  1. Currently, when we create a pv backingstore then the backingstore pod got scheduled on the random basis on any node. This is not good as if any node goes down all the backingstore pod on that node will be down. To increase the performance we have added topology spread constraints to the backingstore pod so they get scheduled on spread basis.

Issues: Fixed #xxx / Gap #xxx

  1. fix: https://bugzilla.redhat.com/show_bug.cgi?id=2294846

Testing Instructions:

  1. After installing noobaa, try creating new pv-backingstore and check if the pv backingstore pod got scheduled and spread across the multiple nodes. Each pod should be spread in different nodes with a maximum skew difference of 1.

…rformance

Signed-off-by: Aayush Chouhan <achouhan@redhat.com>
@aayushchouhan09 aayushchouhan09 requested review from a team and aspandey and removed request for a team July 31, 2024 13:30
"pool": r.BackingStore.Name,
"app": "noobaa",
"pool": r.BackingStore.Name,
"noobaa-backingstore": r.Request.Name,
Copy link
Contributor

Choose a reason for hiding this comment

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

why noobaa-backingstore added?

Copy link
Member Author

Choose a reason for hiding this comment

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

We need to add a label to all the backingstore pods so that topology constraints should work. The current two labels would not work in here so I need to add one. Please correct me if I am wrong. Thanks

// Add topology spread constraints
honor := corev1.NodeInclusionPolicyHonor
if r.PodAgentTemplate.Spec.TopologySpreadConstraints != nil {
r.Logger.Info("TopologySpreadConstraints already exists...")
Copy link
Contributor

Choose a reason for hiding this comment

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

mention it is backing store in log, to make debugging easier.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, I will try to do something better. Thanks

Copy link
Member Author

Choose a reason for hiding this comment

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

We cannot print the backingstore name here, as in this step we are updating the pod template itself for backingstore pod

… more log info

Signed-off-by: Aayush Chouhan <achouhan@redhat.com>
Copy link
Contributor

@naveenpaul1 naveenpaul1 left a comment

Choose a reason for hiding this comment

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

LGTM

@aayushchouhan09 aayushchouhan09 merged commit 2660a9a into noobaa:master Aug 5, 2024
@aayushchouhan09 aayushchouhan09 deleted the topology-const branch September 18, 2024 06:14
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.

3 participants