Skip to content
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

Fix parallel workers under the lead process in the GUI #291

Merged

Conversation

kmoppel-cognite
Copy link
Contributor

Currently they're jumping around annoyingly. Relevant for vers >= 9.6
where parallel query was added

@blogh
Copy link
Collaborator

blogh commented May 10, 2022

I think the sort has to be done in two steps.
We sort in reverse order for the time and the other metrics but we have to sort in ascending order for the pid.

cf https://docs.python.org/3/howto/sorting.html#sort-stability-and-complex-sorts

@blogh blogh requested a review from dlax May 12, 2022 09:06
@kmoppel-cognite
Copy link
Contributor Author

@blogh I think you're right...need some custom sort function...but then...why not to do it in SQL still as I proposed in the first patch - currently actually I think there are no alternative view modes for "Running queries" where rows can be re-ordered in the UI ?

@blogh
Copy link
Collaborator

blogh commented May 19, 2022

@kmoppel-cognite : we can change the sort order while the display is paused. I need to test if it breaks things.

I'll try to look into this next week on the following one.

@blogh
Copy link
Collaborator

blogh commented Jun 8, 2022

So ... we don't sort when paused.
I added a sort step before when we have sortkey.duration anyway.
I think the way parallel processes are handled is just broken ... this will have to do for the moment.

Do you tink it' s ok ?

@blogh
Copy link
Collaborator

blogh commented Jun 8, 2022

... of course I forgot mypy ... after black ...
I really should code more often than once every 5 months ...
fixing it ...

@dlax dlax force-pushed the fix-jumping-parallel-workers-v2 branch from cfc6e42 to 1e2ecf5 Compare June 9, 2022 07:19
@dlax dlax force-pushed the fix-jumping-parallel-workers-v2 branch from 1e2ecf5 to dd41baf Compare June 9, 2022 07:24
Copy link
Member

@dlax dlax left a comment

Choose a reason for hiding this comment

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

Fixed/changed a few minor things based on @blogh commit, rebased and squashed.

LGTM as is; the parallel worker issue can be addressed in a separate changeset.

@dlax dlax merged commit 1b63e42 into dalibo:master Jun 9, 2022
@dlax
Copy link
Member

dlax commented Jun 9, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants