Passive Update mode supported#1014
Open
you-looks-not-tasty wants to merge 1 commit intoiqiyi:develfrom
Open
Conversation
POST /vs/{VipPort}/rs
PUT /vs/{VipPort}
ywc689
added
the
pr/to-review-codes
ywc689
reviewed
Mar 4, 2025
| update = true | ||
| } | ||
|
|
||
| if vsModel != nil { |
Collaborator
There was a problem hiding this comment.
It's the contrary side of previous if condition. if...else... is more efficient.
| update = true | ||
| } | ||
|
|
||
| if len(vsModel.RSs.Items) == len(rss) { |
Collaborator
There was a problem hiding this comment.
It's the contrary side of previous if condition. if...else... is more efficient.
| h.logger.Info("Try update update. rs nat mode has changed.", "VipPort", params.VipPort, "rs", newRs.ID(), "cache nat mode", cacheRs.Spec.Mode, "update nat mode", newRs.GetFwdModeString()) | ||
| update = true | ||
| break | ||
| } |
Collaborator
There was a problem hiding this comment.
Doesn't it need to consider any other field changes in RS?
Comment on lines
+116
to
+119
| newFlags := vs.GetFlags() | ||
| newFlagsNOT := ^newFlags | ||
| tmpFlags := vsModel.RAMFlags ^ newFlagsNOT | ||
| if (tmpFlags & vsModel.RAMFlags) != newFlags { |
Collaborator
There was a problem hiding this comment.
What's purpose of the four line codes? To tell if vsModel.RAMFlags != vs.GetFlags()?
ywc689
removed
the
pr/to-review-codes
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for a new passive update mode that minimizes unnecessary backend updates during high-frequency service changes. Key changes include:
- Introducing a new query parameter "passiveUpdate" in PUT and POST endpoints.
- Updating the embedded API specification to include "passiveUpdate" and adding RAMFlags to support real server snapshot sorting.
- Adjusting the update decision logic in the ipvs command handlers based on the new parameter.
Reviewed Changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/dpvs-agent/restapi/operations/virtualserver/put_vs_vip_port_urlbuilder.go | Added PassiveUpdate field and query parameter generation. |
| tools/dpvs-agent/restapi/operations/virtualserver/put_vs_vip_port_parameters.go | Initialized PassiveUpdate with default value and added binding. |
| tools/dpvs-agent/restapi/operations/virtualserver/post_vs_vip_port_rs_urlbuilder.go | Added PassiveUpdate field and query parameter generation. |
| tools/dpvs-agent/restapi/operations/virtualserver/post_vs_vip_port_rs_parameters.go | Initialized PassiveUpdate with default value and added binding. |
| tools/dpvs-agent/restapi/embedded_spec.go | Updated API spec with PassiveUpdate parameter and added RAMFlags. |
| tools/dpvs-agent/pkg/ipc/types/snapshot.go | New implementation of sort interface for RealServer spec expansion. |
| tools/dpvs-agent/pkg/ipc/types/realserver.go | Introduced SliceRealServerSpec for sorting. |
| tools/dpvs-agent/pkg/ipc/types/getmodel.go | Updated model conversion to include new RAMFlags value. |
| tools/dpvs-agent/models/virtual_server_spec_expand.go | Added RAMFlags to the virtual server spec. |
| tools/dpvs-agent/cmd/ipvs/put_vs_vip_port.go | Updated update logic based on PassiveUpdate and server properties. |
| tools/dpvs-agent/cmd/ipvs/post_vs_vip_port_rs.go | Updated update logic based on PassiveUpdate and real server changes. |
Files not reviewed (1)
- tools/dpvs-agent/dpvs-agent-api.yaml: Language not supported
Comment on lines
+89
to
+96
| h.logger.Info("Try update update. vs not found in snapshot.", "VipPort", params.VipPort) | ||
| update = true | ||
| } | ||
|
|
||
| if vsModel != nil { | ||
| if len(vsModel.RSs.Items) != len(rss) { | ||
| h.logger.Info("Try update update. vs rss len has changed.", "VipPort", params.VipPort) | ||
| update = true |
There was a problem hiding this comment.
The log message contains a duplicated word 'update update'. Consider revising it to a clearer message such as 'Attempt update: vs not found in snapshot.'
Suggested change
| h.logger.Info("Try update update. vs not found in snapshot.", "VipPort", params.VipPort) | |
| update = true | |
| } | |
| if vsModel != nil { | |
| if len(vsModel.RSs.Items) != len(rss) { | |
| h.logger.Info("Try update update. vs rss len has changed.", "VipPort", params.VipPort) | |
| update = true | |
| h.logger.Info("Attempt update: vs not found in snapshot.", "VipPort", params.VipPort) | |
| update = true | |
| } | |
| if vsModel != nil { | |
| if len(vsModel.RSs.Items) != len(rss) { | |
| h.logger.Info("Attempt update: vs rss len has changed.", "VipPort", params.VipPort) |
Comment on lines
+95
to
+96
| h.logger.Info("Try update update. vs rss len has changed.", "VipPort", params.VipPort) | ||
| update = true |
There was a problem hiding this comment.
The log message uses the duplicated phrase 'update update'. Consider revising it to 'Attempt update: vs RS list length has changed.' for clarity.
Suggested change
| h.logger.Info("Try update update. vs rss len has changed.", "VipPort", params.VipPort) | |
| update = true | |
| h.logger.Info("Attempt update: vs RS list length has changed.", "VipPort", params.VipPort) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There will rescheduled to the header realserver while DPVS_SO_SET_ADD | DPVS_SO_SET_EDIT occurred.
The high frequent service changes may lead to backend load imbalance due to schedule resets.
POST /vs/{VipPort}/rs?passiveUpdate=true
PUT /vs/{VipPort}?passiveUpdate=true