Adjust Queue Logging #202
Merged
Adjust Queue Logging #202
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
patricktnast
requested review from
albrja,
collijk,
hussain-jafari,
mattkappel,
rmudambi and
stevebachmeier
as code owners
rmudambi
reviewed
Jan 5, 2024
There was a problem hiding this comment.
Can you include a screenshot of what the logging output looks like when you are able to get scheduled? I'll approve after I see it look as expected.
N.B. - you can use psimulate test sleep rather than running an actual simulation to reduce our cluster impact.
|
Like Rajan, I'd also like to actually see a screenshot of the logging in action. |
| finished_jobs = self._get_finished_jobs() | ||
| start = time.time() | ||
| results = [] | ||
| for job_id in finished_jobs: | ||
| result = self._get_result(job_id) | ||
| if result is not None: | ||
| results.append(result) | ||
| self._logger.info( | ||
| self._logger.debug( |
| ) | ||
| self._status["workers"] = q_workers | ||
| self._status["done"] = 100 * self._status["finished"] / self._status["total"] | ||
| self._status["done"] = ( |
There was a problem hiding this comment.
so "done" for this context is basically finished + successful?
There was a problem hiding this comment.
Yeah, I don't think that the "done" percentage is including failed jobs
|
|
stevebachmeier
approved these changes
Jan 5, 2024
rmudambi
approved these changes
Jan 5, 2024
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.
Adjust Queue Logging
Description
bugfix / documentation
Changes and notes
From the ticket:
As I imply above, thinking about the "jobs" submitted to the cluster is a sort of category error--it's the workers that are really getting submitted. Ultimately, i thought the most sensible thing would be to leave "pending" as-is, under the understanding that this now means just "idle jobs awating workers". Instead, at the "Queue All" level, I added a listing for "inactive workers", that is, the number of "missing workers" from the number that were intially submitted, that are either a) waiting for cluster resources or b) have completed all jobs in the queue and quit themselves.
I don't think there's a lazy way to disambiguate (a) from (b), you'd need to pull information from outside of the registry--and I figured the two situations can be resolved by context (one happens mostly at the beginning of the sim, and one happens mostly at the end).
I too out "workers", assuming the same information is given by "running"
I renamed this job status (but not the underlying FinishedQueue) to Successful for the purposes of logging.
I also changed some log info statements to debug which IMO deserve it
Testing
Tested against nutrition optimization, but I am not actually getting allocated any workers atm