Skip to content

mxqdump: Dump all jobs from group by default without filtering #74

Closed
pmenzel opened this issue Jul 30, 2019 · 6 comments · Fixed by #108
Closed

mxqdump: Dump all jobs from group by default without filtering #74

pmenzel opened this issue Jul 30, 2019 · 6 comments · Fixed by #108

Comments

@pmenzel
Copy link
Contributor

pmenzel commented Jul 30, 2019

Currently, mxqdump -j -g ID will only list running jobs, which is not very clear from the usage information. The default should be changed, that if no --status/-s switch is passed, that all jobs are shown.

@niclas
Copy link
Contributor

niclas commented Aug 27, 2021

mxqsub (headline) or mxqdump (description)?

@pmenzel pmenzel changed the title mxqsub: Dump all jobs from group by default without filtering mxqdump: Dump all jobs from group by default without filtering Aug 27, 2021
@niclas
Copy link
Contributor

niclas commented Aug 27, 2021

To me it looks clear here.

Whether it should be changed is a question for @wwwutz and @donald.

@pmenzel
Copy link
Contributor Author

pmenzel commented Aug 30, 2021

To me it reads like, that only if -s is passed, the default running is used. If no -s is passed, I wouldn’t expect the status to be limited.

@donald
Copy link
Contributor

donald commented Aug 30, 2021

As discussed: Maybe we shouldn't change the current behavior and surprise users who got used to it, but provide a way to list all jobs ( e.g. -g NNN -s all ) and (try to) improve the usage string.

@pmenzel
Copy link
Contributor Author

pmenzel commented Aug 31, 2021

In mxqdump.c there is

mxq/mxqdump.c

Lines 744 to 773 in 66aeeb6

} else if (arg_jobs) {
if (UINT64_HASVALUE(arg_group_id)) {
if (arg_all) {
mx_log_debug("DO: print all jobs in group_id=%lu", arg_group_id);
cnt = dump_jobs(mysql, arg_group_id, UINT64_ALL, UINT64_ALL);
if (!cnt)
mx_log_notice("There are no jobs in group '%lu'.", arg_group_id);
} else {
mx_log_debug("DO: print jobs in group_id=%lu with status='%s(%lu)'",
arg_group_id, mxq_job_status_to_name(arg_status), arg_status);
cnt = dump_jobs(mysql, arg_group_id, arg_status, UINT64_ALL);
if (!cnt)
mx_log_notice("There are no jobs with status '%s' in group '%lu'.", mxq_job_status_to_name(arg_status), arg_group_id);
}
} else {
if (arg_all) {
mx_log_debug("DO: print all running jobs");
dump_jobs(mysql, UINT64_ALL, MXQ_JOB_STATUS_RUNNING, UINT64_ALL);
} else {
mx_log_debug("DO: print MY running jobs");
cnt = dump_jobs(mysql, UINT64_ALL, MXQ_JOB_STATUS_RUNNING, arg_uid);
if (!cnt) {
if (arg_uid == ruid)
mx_log_notice("You do not have any jobs running on the cluster.");
else
mx_log_notice("No running jobs for user with uid '%lu'.", arg_uid);
}
}
}

suggesting, that -a can also be used with jobs-options. When -g/--group-id is specified status is unfiltered. When -u/--user is specified the jobs are filtered on status running again. Very confusing.

@donald
Copy link
Contributor

donald commented Aug 31, 2021

  • Dump all jobs from group by default without filtering
  • We said, no we won't change the default, but provide a way to dump all jobs from group
  • @niclas discovered that that optional already exists, and so just updated the usage to document it.

So is the issue fixed now? Does the usage show how to dump all jobs of a group and does it work?

I don't understand the last comment. Is that part of the "dump all jobs from group" Problem or a new issue? Are there other "hidden" options which need to be documented in the usage string? Is just the code ugly?

Very confusing.

Agreed.

pmenzel added a commit that referenced this issue Sep 1, 2021
With `mxqdump`, currently, reading the usage, it’s not clear how to dump
all jobs in a group without any filtering on the status, which defaults
to *running*. The code actually check if `arg_all` is set, meaning the
switch `-a`/`--all` is passed. So, document that in the usage.

Instead of `-a`/`--all` also `--status=all` or `--status=any` would be
possible, but also take a different code path. So, only document
`-a`/`--all`.

Passing `--debug` shows the code paths taken:

    $ mxqdump --debug --jobs -g 472581 -a
    mxqdump mxqdump.c:737:main(): MySQL: Connection to database established.
    mxqdump mxqdump.c:747:main(): DEBUG: DO: print all jobs in group_id=472581
    […]
    $ mxqdump --debug --jobs -g 472581 --status=all # or any
    mxqdump mxqdump.c:737:main(): MySQL: Connection to database established.
    mxqdump mxqdump.c:754:main(): DEBUG: DO: print jobs in group_id=472581 with status='invalid(18446744073709551614)'
    […]

Fixes: #74
pmenzel added a commit that referenced this issue Sep 1, 2021
With `mxqdump`, currently, reading the usage, it’s not clear how to dump
all jobs in a group without any filtering on the status, which defaults
to *running*. The code actually check if `arg_all` is set, meaning the
switch `-a`/`--all` is passed. So, document that in the usage.

Instead of `-a`/`--all` also `--status=all` or `--status=any` would be
possible, but also takes a different code path. As that is invisible to
the user, mention it as an alias.

Passing `--debug` shows the code paths taken:

    $ mxqdump --debug --jobs -g 472581 -a
    mxqdump mxqdump.c:737:main(): MySQL: Connection to database established.
    mxqdump mxqdump.c:747:main(): DEBUG: DO: print all jobs in group_id=472581
    […]
    $ mxqdump --debug --jobs -g 472581 --status=all # or any
    mxqdump mxqdump.c:737:main(): MySQL: Connection to database established.
    mxqdump mxqdump.c:754:main(): DEBUG: DO: print jobs in group_id=472581 with status='invalid(18446744073709551614)'
    […]

Fixes: #74
Sign in to join this conversation on GitHub.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants