Use scheduling info instead of the whole desiredLRP #79
Conversation
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
PlamenDoychev
requested changes
Jan 15, 2024
| desiredLRP, err := h.desiredLRPDB.DesiredLRPByProcessGuid(ctx, logger, actualLRPKey.ProcessGuid) | ||
| if err != nil { | ||
| logger.Error("failed-fetching-desired-lrp", err) | ||
| schedInfos, err := h.desiredLRPDB.DesiredLRPSchedulingInfos(ctx, logger, models.DesiredLRPFilter{ProcessGuids: []string{actualLRPKey.ProcessGuid}}) |
There was a problem hiding this comment.
What about exposing a method like DesiredLRPSchedulingInfoByProcessGuid in the desired_lrp_db interface, so that we can get rid of the array length validation?
klapkov
commented
Jan 16, 2024
klapkov
commented
Jan 16, 2024
Casing / more specific description
No need to specify VolumeMount type process_guid -> receivedProcessGuid for clarity Typo on `errros`
ebroberson
reviewed
Feb 29, 2024
ebroberson
approved these changes
Feb 29, 2024
PlamenDoychev
added a commit
to PlamenDoychev/community
that referenced
this pull request
Mar 9, 2024
[DIEGO-RELEASE] Add locket client keepalive time and timeout to jobs: cloudfoundry/diego-release#722 [DIEGO-RELEASE] Make max_containers prop configurable: cloudfoundry/diego-release#876 [BBS] Add request metrics for BBS - still open: cloudfoundry/bbs#80 [BBS] Use scheduling info instead of the whole desiredLRP: cloudfoundry/bbs#79 [BBS] Add routing info endpoint: cloudfoundry/bbs#66 [BBS] Remove cpu_weight limit: cloudfoundry/bbs#81 [EXECUTOR] Improve error handling on process start - still open: cloudfoundry/executor#91 [LOCKET] Add a keepalive timeout on the locket client: cloudfoundry/locket#12 [GROOTFS] UsedVolumesSize and UsedStoreInBytes metrics: cloudfoundry/grootfs#155 [ROUTE-EMITTER] Use routing info bbs endpoint when syncing: cloudfoundry/route-emitter#23 [ROUTE-EMITTER] Use routing_info for desired_lrp's when there are missing actual_lrp's: cloudfoundry/route-emitter#26 [SILK-RELEASE] Deduplicate Iptables Rules with Dynamic ASG's: cloudfoundry/silk-release#101 [SILK-RELEASE] Make container_metadata_file_check_timeout on silk-shutdown configurable: cloudfoundry/silk-release#111
This was referenced Mar 9, 2024
ameowlia
pushed a commit
to cloudfoundry/community
that referenced
this pull request
Mar 12, 2024
[DIEGO-RELEASE] Add locket client keepalive time and timeout to jobs: cloudfoundry/diego-release#722 [DIEGO-RELEASE] Make max_containers prop configurable: cloudfoundry/diego-release#876 [BBS] Add request metrics for BBS - still open: cloudfoundry/bbs#80 [BBS] Use scheduling info instead of the whole desiredLRP: cloudfoundry/bbs#79 [BBS] Add routing info endpoint: cloudfoundry/bbs#66 [BBS] Remove cpu_weight limit: cloudfoundry/bbs#81 [EXECUTOR] Improve error handling on process start - still open: cloudfoundry/executor#91 [LOCKET] Add a keepalive timeout on the locket client: cloudfoundry/locket#12 [GROOTFS] UsedVolumesSize and UsedStoreInBytes metrics: cloudfoundry/grootfs#155 [ROUTE-EMITTER] Use routing info bbs endpoint when syncing: cloudfoundry/route-emitter#23 [ROUTE-EMITTER] Use routing_info for desired_lrp's when there are missing actual_lrp's: cloudfoundry/route-emitter#26 [SILK-RELEASE] Deduplicate Iptables Rules with Dynamic ASG's: cloudfoundry/silk-release#101 [SILK-RELEASE] Make container_metadata_file_check_timeout on silk-shutdown configurable: cloudfoundry/silk-release#111 wip
ameowlia
pushed a commit
to cloudfoundry/community
that referenced
this pull request
Mar 12, 2024
[DIEGO-RELEASE] Add locket client keepalive time and timeout to jobs: cloudfoundry/diego-release#722 [DIEGO-RELEASE] Make max_containers prop configurable: cloudfoundry/diego-release#876 [BBS] Add request metrics for BBS - still open: cloudfoundry/bbs#80 [BBS] Use scheduling info instead of the whole desiredLRP: cloudfoundry/bbs#79 [BBS] Add routing info endpoint: cloudfoundry/bbs#66 [BBS] Remove cpu_weight limit: cloudfoundry/bbs#81 [EXECUTOR] Improve error handling on process start - still open: cloudfoundry/executor#91 [LOCKET] Add a keepalive timeout on the locket client: cloudfoundry/locket#12 [GROOTFS] UsedVolumesSize and UsedStoreInBytes metrics: cloudfoundry/grootfs#155 [ROUTE-EMITTER] Use routing info bbs endpoint when syncing: cloudfoundry/route-emitter#23 [ROUTE-EMITTER] Use routing_info for desired_lrp's when there are missing actual_lrp's: cloudfoundry/route-emitter#26 [SILK-RELEASE] Deduplicate Iptables Rules with Dynamic ASG's: cloudfoundry/silk-release#101 [SILK-RELEASE] Make container_metadata_file_check_timeout on silk-shutdown configurable: cloudfoundry/silk-release#111 wip
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Related to cloudfoundry/diego-release#883. Uses DesiredLRPSchedulingInfos where we do not need the whole desiredLRP. We have some performance to be gained here.