-
Notifications
You must be signed in to change notification settings - Fork 110
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
Rename set_priority to upgrade_priority #1112
Merged
adam-singer
merged 1 commit into
TraceMachina:dev
from
adam-singer:adams/set-priority-to-upgrade-priority
Jul 8, 2024
Merged
Rename set_priority to upgrade_priority #1112
adam-singer
merged 1 commit into
TraceMachina:dev
from
adam-singer:adams/set-priority-to-upgrade-priority
Jul 8, 2024
Conversation
This file contains 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
allada
approved these changes
Jul 8, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: complete! 1 of 1 LGTMs obtained
allada
added a commit
to allada/nativelink-fork
that referenced
this pull request
Jul 16, 2024
This is an extremely significant overhaul to Nativelink's scheduler component. This new scheduler design is to enable a distributed scheduling system. The new components & definitions: * AwaitedActionDb - An interface that is easier to work with when dealing with key-value storage systems. * MemoryAwaitedActionDb - An in-memory set of hashmaps & btrees used to satisfy the requirements of AwaitedActionDb interface. * ClientStateManager - A minimal interface required to satisfy the requirements of a client-facing scheduler. * WorkerStateManager - A minimal interface required to satisfy the requirements of a worker-facing scheduler. * MatchingEngineStateManager - A minimal interface required to satisfy a the engine that matches queued jobs to workers. * SimpleSchedulerStateManager - An implementation that satisfies ClientStateManager, WorkerStateManager & MatchingEngineStateManager with all the logic of the previous "SimpleScheduler" logic moved behind each interface. * ApiWorkerScheduler - A component that handles all knowledge about workers state and implmenets the WorkerScheduler interface and translates them into the WorkerStateManager interface. * SimpleScheduler - Translation calls of the ClientScheduler interface into ClientStateManager & MatchingEngineStateManager. This component is currently always fowards calls to SimpleSchedulerStateManager then to MemoryAwaitedActionDb. Future changes will make these inner components dynamic via config. In addition we have hardened the interactions of different kind of IDs in NativeLink. Most relivent is the separation & introduction of: * OperationId - Represents an individal operation being requested to be executed that is unique across all of time. * ClientOperationId - An ID issued to the client when the client requests to execute a job. This ID will point to an OperationId internally, but the client is never exposed to the OperationId. * AwaitedActionHashKey - A key used to uniquely identify an action that is not unique across time. This means that this key might have multiple OperationId's that have executed it across different points in time. This key is used as a "fingerprint" of an operation that the client wants to execute and the scheduler may decide to join the stream onto an existing operation if this key has a hit. Overall these changes pave the way for more robust scheduler implementations, most notably, distributed scheduler implementations will be easier to impelemnt and will be introduced in followup PRs. This commit was developed on a side branch and consisted of the following commits with corresponding code reviews: 54ed73c Add scheduler metrics back (TraceMachina#1171) 50fdbd7 fix formatting (TraceMachina#1170) 8926236 Merge in main and format (TraceMachina#1168) 9c2c7b9 key as u64 (TraceMachina#1166) 0192051 Cleanup unused code and comments (TraceMachina#1165) 080df5d Add versioning to AwaitedAction (TraceMachina#1163) 73c19c4 Fix sequence bug in new memory store manager (TraceMachina#1162) 6e50d2c New AwaitedActionDb implementation (TraceMachina#1157) 18db991 Fix test on running_actions_manager_test (TraceMachina#1141) e50ef3c Rename workers to `worker_scheduler` 1fdd505 SimpleScheduler now uses config for action pruning (TraceMachina#1137) eaaa872 Change encoding for items that are cachable (TraceMachina#1136) d647056 Errors are now properly handles in subscription (TraceMachina#1135) 7c3e730 Restructure files to be more appropriate (TraceMachina#1131) 5e98ec9 ClientAwaitedAction now uses a channel to notify drops happened (TraceMachina#1130) 52beaf9 Cleanup unused structs (TraceMachina#1128) e86fe08 Remove all uses of salt and put under ActionUniqueQualifier (TraceMachina#1126) 3b86036 Remove all need for workers to know about ActionId (TraceMachina#1125) 5482d7f Fix bazel build and test on dev (TraceMachina#1123) ba52c7f Implement get_action_info to all ActionStateResult impls (TraceMachina#1118) 2fa4fee Remove MatchingEngineStateManager::remove_operation (TraceMachina#1119) 34dea06 Remove unused proto field (TraceMachina#1117) 3070a40 Remove metrics from new scheduler (TraceMachina#1116) e95adfc StateManager will now cleanup actions on client disconnect (TraceMachina#1107) 6f8c001 Fix worker execution issues (TraceMachina#1114) d353c30 rename set_priority to upgrade_priority (TraceMachina#1112) 0d93671 StateManager can now be notified of noone listeneing (TraceMachina#1093) cfc0cf6 ActionScheduler will now use ActionListener instead of tokio::watch (TraceMachina#1091) d70d31d QA fixes for scheduler-v2 (TraceMachina#1092) f2cea0c [Refactor] Complete rewrite of SimpleScheduler 34d93b7 [Refactor] Move worker notification in SimpleScheduler under Workers b9d9702 [Refactor] Moves worker logic back to SimpleScheduler 7a16e2e [Refactor] Move scheduler state behind mute
allada
added a commit
to allada/nativelink-fork
that referenced
this pull request
Jul 16, 2024
This is an extremely significant overhaul to Nativelink's scheduler component. This new scheduler design is to enable a distributed scheduling system. The new components & definitions: * AwaitedActionDb - An interface that is easier to work with when dealing with key-value storage systems. * MemoryAwaitedActionDb - An in-memory set of hashmaps & btrees used to satisfy the requirements of AwaitedActionDb interface. * ClientStateManager - A minimal interface required to satisfy the requirements of a client-facing scheduler. * WorkerStateManager - A minimal interface required to satisfy the requirements of a worker-facing scheduler. * MatchingEngineStateManager - A minimal interface required to satisfy a engine that matches queued jobs to workers. * SimpleSchedulerStateManager - An implements that satisfies ClientStateManager, WorkerStateManager & MatchingEngineStateManager with all the logic of the previous "SimpleScheduler" logic moved behind each interface. * ApiWorkerScheduler - A component that handles all knowledge about workers state and implmenets the WorkerScheduler interface and translates them into the WorkerStateManager interface. * SimpleScheduler - Translation calls of the ClientScheduler interface into ClientStateManager & MatchingEngineStateManager. This component is currently always forwards calls to SimpleSchedulerStateManager then to MemoryAwaitedActionDb. Future changes will make these inner components dynamic via config. In addition we have hardened the interactions of different kind of IDs in NativeLink. Most relevant is the separation & introduction of: * OperationId - Represents an individual operation being requested to be executed that is unique across all of time. * ClientOperationId - An ID issued to the client when the client requests to execute a job. This ID will point to an OperationId internally, but the client is never exposed to the OperationId. * AwaitedActionHashKey - A key used to uniquely identify an action that is not unique across time. This means that this key might have multiple OperationId's that have executed it across different points in time. This key is used as a "fingerprint" of an operation that the client wants to execute and the scheduler may decide to join the stream onto an existing operation if this key has a hit. Overall these changes pave the way for more robust scheduler implementations, most notably, distributed scheduler implementations will be easier to implement and will be introduced in followup PRs. This commit was developed on a side branch and consisted of the following commits with corresponding code reviews: 54ed73c Add scheduler metrics back (TraceMachina#1171) 50fdbd7 fix formatting (TraceMachina#1170) 8926236 Merge in main and format (TraceMachina#1168) 9c2c7b9 key as u64 (TraceMachina#1166) 0192051 Cleanup unused code and comments (TraceMachina#1165) 080df5d Add versioning to AwaitedAction (TraceMachina#1163) 73c19c4 Fix sequence bug in new memory store manager (TraceMachina#1162) 6e50d2c New AwaitedActionDb implementation (TraceMachina#1157) 18db991 Fix test on running_actions_manager_test (TraceMachina#1141) e50ef3c Rename workers to `worker_scheduler` 1fdd505 SimpleScheduler now uses config for action pruning (TraceMachina#1137) eaaa872 Change encoding for items that are cachable (TraceMachina#1136) d647056 Errors are now properly handles in subscription (TraceMachina#1135) 7c3e730 Restructure files to be more appropriate (TraceMachina#1131) 5e98ec9 ClientAwaitedAction now uses a channel to notify drops happened (TraceMachina#1130) 52beaf9 Cleanup unused structs (TraceMachina#1128) e86fe08 Remove all uses of salt and put under ActionUniqueQualifier (TraceMachina#1126) 3b86036 Remove all need for workers to know about ActionId (TraceMachina#1125) 5482d7f Fix bazel build and test on dev (TraceMachina#1123) ba52c7f Implement get_action_info to all ActionStateResult impls (TraceMachina#1118) 2fa4fee Remove MatchingEngineStateManager::remove_operation (TraceMachina#1119) 34dea06 Remove unused proto field (TraceMachina#1117) 3070a40 Remove metrics from new scheduler (TraceMachina#1116) e95adfc StateManager will now cleanup actions on client disconnect (TraceMachina#1107) 6f8c001 Fix worker execution issues (TraceMachina#1114) d353c30 rename set_priority to upgrade_priority (TraceMachina#1112) 0d93671 StateManager can now be notified of noone listeneing (TraceMachina#1093) cfc0cf6 ActionScheduler will now use ActionListener instead of tokio::watch (TraceMachina#1091) d70d31d QA fixes for scheduler-v2 (TraceMachina#1092) f2cea0c [Refactor] Complete rewrite of SimpleScheduler 34d93b7 [Refactor] Move worker notification in SimpleScheduler under Workers b9d9702 [Refactor] Moves worker logic back to SimpleScheduler 7a16e2e [Refactor] Move scheduler state behind mute
allada
added a commit
that referenced
this pull request
Jul 16, 2024
This is a significant overhaul to Nativelink's scheduler component. This new scheduler design is to enable a distributed scheduling system. The new components & definitions: * AwaitedActionDb - An interface that is easier to work with when dealing with key-value storage systems. * MemoryAwaitedActionDb - An in-memory set of hashmaps & btrees used to satisfy the requirements of AwaitedActionDb interface. * ClientStateManager - A minimal interface required to satisfy the requirements of a client-facing scheduler. * WorkerStateManager - A minimal interface required to satisfy the requirements of a worker-facing scheduler. * MatchingEngineStateManager - A minimal interface required to satisfy a engine that matches queued jobs to workers. * SimpleSchedulerStateManager - An implements that satisfies ClientStateManager, WorkerStateManager & MatchingEngineStateManager with all the logic of the previous "SimpleScheduler" logic moved behind each interface. * ApiWorkerScheduler - A component that handles all knowledge about workers state and implmenets the WorkerScheduler interface and translates them into the WorkerStateManager interface. * SimpleScheduler - Translation calls of the ClientScheduler interface into ClientStateManager & MatchingEngineStateManager. This component is currently always forwards calls to SimpleSchedulerStateManager then to MemoryAwaitedActionDb. Future changes will make these inner components dynamic via config. In addition we have hardened the interactions of different kind of IDs in NativeLink. Most relevant is the separation & introduction of: * OperationId - Represents an individual operation being requested to be executed that is unique across all of time. * ClientOperationId - An ID issued to the client when the client requests to execute a job. This ID will point to an OperationId internally, but the client is never exposed to the OperationId. * AwaitedActionHashKey - A key used to uniquely identify an action that is not unique across time. This means that this key might have multiple OperationId's that have executed it across different points in time. This key is used as a "fingerprint" of an operation that the client wants to execute and the scheduler may decide to join the stream onto an existing operation if this key has a hit. Overall these changes pave the way for more robust scheduler implementations, most notably, distributed scheduler implementations will be easier to implement and will be introduced in followup PRs. This commit was developed on a side branch and consisted of the following commits with corresponding code reviews: 54ed73c Add scheduler metrics back (#1171) 50fdbd7 fix formatting (#1170) 8926236 Merge in main and format (#1168) 9c2c7b9 key as u64 (#1166) 0192051 Cleanup unused code and comments (#1165) 080df5d Add versioning to AwaitedAction (#1163) 73c19c4 Fix sequence bug in new memory store manager (#1162) 6e50d2c New AwaitedActionDb implementation (#1157) 18db991 Fix test on running_actions_manager_test (#1141) e50ef3c Rename workers to `worker_scheduler` 1fdd505 SimpleScheduler now uses config for action pruning (#1137) eaaa872 Change encoding for items that are cachable (#1136) d647056 Errors are now properly handles in subscription (#1135) 7c3e730 Restructure files to be more appropriate (#1131) 5e98ec9 ClientAwaitedAction now uses a channel to notify drops happened (#1130) 52beaf9 Cleanup unused structs (#1128) e86fe08 Remove all uses of salt and put under ActionUniqueQualifier (#1126) 3b86036 Remove all need for workers to know about ActionId (#1125) 5482d7f Fix bazel build and test on dev (#1123) ba52c7f Implement get_action_info to all ActionStateResult impls (#1118) 2fa4fee Remove MatchingEngineStateManager::remove_operation (#1119) 34dea06 Remove unused proto field (#1117) 3070a40 Remove metrics from new scheduler (#1116) e95adfc StateManager will now cleanup actions on client disconnect (#1107) 6f8c001 Fix worker execution issues (#1114) d353c30 rename set_priority to upgrade_priority (#1112) 0d93671 StateManager can now be notified of noone listeneing (#1093) cfc0cf6 ActionScheduler will now use ActionListener instead of tokio::watch (#1091) d70d31d QA fixes for scheduler-v2 (#1092) f2cea0c [Refactor] Complete rewrite of SimpleScheduler 34d93b7 [Refactor] Move worker notification in SimpleScheduler under Workers b9d9702 [Refactor] Moves worker logic back to SimpleScheduler 7a16e2e [Refactor] Move scheduler state behind mute
allada
added a commit
that referenced
this pull request
Jul 17, 2024
This is a significant overhaul to Nativelink's scheduler component. This new scheduler design is to enable a distributed scheduling system. The new components & definitions: * AwaitedActionDb - An interface that is easier to work with when dealing with key-value storage systems. * MemoryAwaitedActionDb - An in-memory set of hashmaps & btrees used to satisfy the requirements of AwaitedActionDb interface. * ClientStateManager - A minimal interface required to satisfy the requirements of a client-facing scheduler. * WorkerStateManager - A minimal interface required to satisfy the requirements of a worker-facing scheduler. * MatchingEngineStateManager - A minimal interface required to satisfy a engine that matches queued jobs to workers. * SimpleSchedulerStateManager - An implements that satisfies ClientStateManager, WorkerStateManager & MatchingEngineStateManager with all the logic of the previous "SimpleScheduler" logic moved behind each interface. * ApiWorkerScheduler - A component that handles all knowledge about workers state and implmenets the WorkerScheduler interface and translates them into the WorkerStateManager interface. * SimpleScheduler - Translation calls of the ClientScheduler interface into ClientStateManager & MatchingEngineStateManager. This component is currently always forwards calls to SimpleSchedulerStateManager then to MemoryAwaitedActionDb. Future changes will make these inner components dynamic via config. In addition we have hardened the interactions of different kind of IDs in NativeLink. Most relevant is the separation & introduction of: * OperationId - Represents an individual operation being requested to be executed that is unique across all of time. * ClientOperationId - An ID issued to the client when the client requests to execute a job. This ID will point to an OperationId internally, but the client is never exposed to the OperationId. * AwaitedActionHashKey - A key used to uniquely identify an action that is not unique across time. This means that this key might have multiple OperationId's that have executed it across different points in time. This key is used as a "fingerprint" of an operation that the client wants to execute and the scheduler may decide to join the stream onto an existing operation if this key has a hit. Overall these changes pave the way for more robust scheduler implementations, most notably, distributed scheduler implementations will be easier to implement and will be introduced in followup PRs. This commit was developed on a side branch and consisted of the following commits with corresponding code reviews: 54ed73c Add scheduler metrics back (#1171) 50fdbd7 fix formatting (#1170) 8926236 Merge in main and format (#1168) 9c2c7b9 key as u64 (#1166) 0192051 Cleanup unused code and comments (#1165) 080df5d Add versioning to AwaitedAction (#1163) 73c19c4 Fix sequence bug in new memory store manager (#1162) 6e50d2c New AwaitedActionDb implementation (#1157) 18db991 Fix test on running_actions_manager_test (#1141) e50ef3c Rename workers to `worker_scheduler` 1fdd505 SimpleScheduler now uses config for action pruning (#1137) eaaa872 Change encoding for items that are cachable (#1136) d647056 Errors are now properly handles in subscription (#1135) 7c3e730 Restructure files to be more appropriate (#1131) 5e98ec9 ClientAwaitedAction now uses a channel to notify drops happened (#1130) 52beaf9 Cleanup unused structs (#1128) e86fe08 Remove all uses of salt and put under ActionUniqueQualifier (#1126) 3b86036 Remove all need for workers to know about ActionId (#1125) 5482d7f Fix bazel build and test on dev (#1123) ba52c7f Implement get_action_info to all ActionStateResult impls (#1118) 2fa4fee Remove MatchingEngineStateManager::remove_operation (#1119) 34dea06 Remove unused proto field (#1117) 3070a40 Remove metrics from new scheduler (#1116) e95adfc StateManager will now cleanup actions on client disconnect (#1107) 6f8c001 Fix worker execution issues (#1114) d353c30 rename set_priority to upgrade_priority (#1112) 0d93671 StateManager can now be notified of noone listeneing (#1093) cfc0cf6 ActionScheduler will now use ActionListener instead of tokio::watch (#1091) d70d31d QA fixes for scheduler-v2 (#1092) f2cea0c [Refactor] Complete rewrite of SimpleScheduler 34d93b7 [Refactor] Move worker notification in SimpleScheduler under Workers b9d9702 [Refactor] Moves worker logic back to SimpleScheduler 7a16e2e [Refactor] Move scheduler state behind mute
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.
Update naming and documentation around upgrading priority
This change is