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

processes sort with parallel worker is broken #297

Closed
dlax opened this issue Jun 9, 2022 · 4 comments · Fixed by #309
Closed

processes sort with parallel worker is broken #297

dlax opened this issue Jun 9, 2022 · 4 comments · Fixed by #309
Labels

Comments

@dlax
Copy link
Member

dlax commented Jun 9, 2022

See discussion at #291.

From postgres 13, maybe we could rely on leader_pid column of pg_stat_activity view as first sort step on SQL side?

@kmoppel-cognite
Copy link
Contributor

FYI - just remembered that I've seen actually cases where parent pid was higher than background worker / child pid...so this code here can deliver wrong results still https://github.com/dalibo/pg_activity/pull/291/files#diff-c7ba7b4a3a801c37989b9ac81ece8995e3154d2f67325272f24877b5e730cf65R219-R222

@blogh
Copy link
Collaborator

blogh commented Jun 9, 2022

FYI - just remembered that I've seen actually cases where parent pid was higher than
background worker / child pid

I agree, I've seen it too. I think changing the sort order when paused could be a valuable
thing to have in the future in which case sorting in the SQL will not help. (I'd also like to add
filters when paused).

When we order by mem / cpu / read / write, the \_ thing is also meaningless because
the worker process is often not listed after their parent.

It was thinking about adding the parent pid and the backend type info in the UI so that we
have a parent pid to sort on in the sorted function. But it adds a lot of text and we would
need a better way to filter what we display. Some of the ideas in #260 might help for that
(group columns and allow to cycle between groups).

It would also be neat to group the leader process and it's childs in a single "ui block", but
it's a lot more complex.

Before changing the output, the switch to "rich" must be finished or cancelled. I'll try to
help there next.

@blogh
Copy link
Collaborator

blogh commented Jun 9, 2022

@kmoppel-cognite what do you think about sorting/filtering in paused/interactive mode ?

@kmoppel-cognite
Copy link
Contributor

@kmoppel-cognite what do you think about sorting/filtering in paused/interactive mode ?

@blogh For me personally I've never had the need for such extra features, I've just used "pause" to freeze the UI to be able to copy some SQL or kill some PIDs. And there's also the new --filter syntax, so I'd personally KISS and try to reduce UI logic / code as much as possible.

@blogh blogh closed this as completed in #309 Sep 7, 2022
blogh added a commit to blogh/pg_activity that referenced this issue Sep 16, 2022
Breaking change:

* Attr 18.1 is required

Change log:

* Add more information to the header (instance and process stats) (Tests
  by @Krysztophe)
* Add the --refresh option to the cli to set the refresh rate (dalibo#293)
  (Requested by @crysman)
* Add the --debug-file option to enable logging (still mostly unused)
* Add hints about runtime disabled features (dalibo#300) (Reported by @rutchkiwi)
* The SUPERUSER privilege is not longer required (dalibo#277) (Requested by
  @Raymondmax)

Bug fixes:

* Add the --no-walreceiver to disable wal receiver stats for Aurora
  (dalibo#301) (Reported by @grutz)
* Add the --no-tempfiles option to disable temp file statistics and
  add it to the --rds command (dalibo#303) (Reported by @adityabaradwaj)
* Fix server information queries for v12/v13 (Reported and fixed by
  @kmoppel-cognite)
* Fix InvalidTextRepresentation errors (dalibo#275) (Fix proposed by
  @ssharunas)
* Fix sort order for parallel queries (dalibo#297) (Reported and fixed by
  @kmoppel-cognite)
* Doc fixes and packaging improvements (@kianmeng, @Vampouille)
@blogh blogh mentioned this issue Sep 16, 2022
blogh added a commit that referenced this issue Sep 16, 2022
Breaking change:

* Attr 18.1 is required

Change log:

* Add more information to the header (instance and process stats) (Tests
  by @Krysztophe)
* Add the --refresh option to the cli to set the refresh rate (#293)
  (Requested by @crysman)
* Add the --debug-file option to enable logging (still mostly unused)
* Add hints about runtime disabled features (#300) (Reported by @rutchkiwi)
* The SUPERUSER privilege is not longer required (#277) (Requested by
  @Raymondmax)

Bug fixes:

* Add the --no-walreceiver to disable wal receiver stats for Aurora
  (#301) (Reported by @grutz)
* Add the --no-tempfiles option to disable temp file statistics and
  add it to the --rds command (#303) (Reported by @adityabaradwaj)
* Fix server information queries for v12/v13 (Reported and fixed by
  @kmoppel-cognite)
* Fix InvalidTextRepresentation errors (#275) (Fix proposed by
  @ssharunas)
* Fix sort order for parallel queries (#297) (Reported and fixed by
  @kmoppel-cognite)
* Doc fixes and packaging improvements (@kianmeng, @Vampouille)
blogh added a commit to blogh/pg_activity that referenced this issue Sep 16, 2022
Breaking change:

* Attr 18.1 is required

Change log:

* Add more information to the header (instance and process stats) (Tests
  by @Krysztophe)
* Add the --refresh option to the cli to set the refresh rate (dalibo#293)
  (Requested by @crysman)
* Add the --debug-file option to enable logging (still mostly unused)
* Add hints about runtime disabled features (dalibo#300) (Reported by @rutchkiwi)
* The SUPERUSER privilege is not longer required (dalibo#277) (Requested by
  @Raymondmax)

Bug fixes:

* Add the --no-walreceiver to disable wal receiver stats for Aurora
  (dalibo#301) (Reported by @grutz)
* Add the --no-tempfiles option to disable temp file statistics and
  add it to the --rds command (dalibo#303) (Reported by @adityabaradwaj)
* Fix server information queries for v12/v13 (Reported and fixed by
  @kmoppel-cognite)
* Fix InvalidTextRepresentation errors (dalibo#275) (Fix proposed by
  @ssharunas)
* Fix sort order for parallel queries (dalibo#297) (Reported and fixed by
  @kmoppel-cognite)
* Doc fixes and packaging improvements (@kianmeng, @Vampouille)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants