index: resolve owner names in SQL; fix path defaulting, ls <file>, and config errors#344
Merged
Merged
Conversation
…d config errors
Owner/group name resolution moves into the GUFI query, plus a set of
path-handling, formatting, and config-validation fixes for the `beegfs
index` subcommands.
- uid/gid resolution in SQL: ls/find/stat now select uidtouser(uid)/
gidtogroup(gid) in their output templates instead of resolving in Go via
os/user. The CGO-disabled binary's os/user only reads /etc/passwd and
/etc/group, so NSS-backed (LDAP/SSSD) names never resolved and showed as
numbers (e.g. gid 5900 instead of "thinparq"). The GUFI helpers run in
the index process (glibc -> NSS), so names resolve on the index host,
matching how a remote index already resolves them. The Go lookupUID/
lookupGID helpers are removed. Aggregate INSERT sources stay numeric;
FindAggGSelect resolves at output.
- stat formatting: mode is rendered octal/symbolic (0644/-rw-r--r--) and
uid/gid as num/name (1000/alice), matching gufi_stat. The resolved name
columns are selected next to uid/gid.
- default path for stats/info/query: an omitted path now defaults to "."
(which resolves to the mount root / whole index) instead of the cwd. The
cwd default failed with "not under index root" when run outside the
mount, notably for a remote index where the mount-anchoring step is
skipped. This matches ls/find/stat.
- ls <file>: a file argument is now listed from its parent directory with
an exact name match (new LsTargetIsFile/LsFile), like ls(1)/gufi_ls,
instead of returning nothing. File-vs-dir is detected locally with a
cheap stat and, for a remote index, by probing the path's own summary
row over gufi_query. The cmd also checks the parent index dir so a file
path no longer trips "no index found".
- stats median: median-leaf-* reuse GUFI's median() aggregate instead of a
hand-rolled AVG over ORDER BY/LIMIT/OFFSET.
- .beegfs.index validation: a malformed or unreadable file is now a hard
error, raised once in the index command's PersistentPreRunE, naming the
expected single [index] table. Previously it was logged at Warn and
silently ignored, then surfaced confusingly later as "index root not
set". loadDotIndex/DotIndexPath return the error; ApplyDotIndexOverrides
leaves cfg unchanged on a bad file (the PreRun check has already
reported it).
- ls --skip: clarify the flag help (FILE lists directory basenames to
prune, not a path to skip)
…d info gaps
Three changes from the resolver-branch review.
1. Selective owner-name resolution (find/ls). The templates selected
uidtouser(uid)/gidtogroup(gid) in every query, but GUFI resolves those
with an uncached getpwuid_r/getgrgid_r per row while the user/group
columns are displayed only with -l/--columns/debug. A default listing of
a large tree therefore ran a name lookup per row and discarded it. The
templates now take an owner-column fragment (ownerCols) that is numeric
by default and resolved only when the column will actually be shown
(ownerNamesRequested honors -l, --columns, and debug). Resolution still
happens in SQL on the index host, so it stays correct for a remote index.
stat is unchanged (single row, always shows id/name).
2. Remote stat no longer masks failures. The directory-summary branch
discarded the row-stream wait error and fell through to the file spec
whenever it was non-nil, so a genuine ssh/SQL/parse error was reported as
a confusing "no index entry found". A non-directory path exits cleanly
with no rows, so only an empty result falls through now; a real error is
returned.
3. index info flags a missing tree summary. When no summary row exists
(e.g. --skip-treesummary) the stats columns render "-" instead of blank
cells, distinguishing "not computed" from zero.
…ns all
Two output fixes to the index CLI:
- ls of a nonexistent path exited 0 silently instead of erroring. The
ls <file> support routes a file/name target through LsFile, but a typo'd
or deleted path takes that same path and matches no rows; with no
not-found check the command printed nothing and returned success. Error
when a file target (isFile) matches zero rows, mirroring stat. An empty
directory takes the Ls path (isFile==false) and still lists as before.
- --columns all displayed numeric uid/gid. ownerNamesRequested enabled name
resolution only for an explicit "user"/"group" column, but cmdfmt expands
"all" to every column, so the owner columns were shown yet left numeric.
Treat "all" as requesting owner names.
iamjoemccormick
requested changes
Jul 1, 2026
iamjoemccormick
left a comment
iamjoemccormick
left a comment
Member
There was a problem hiding this comment.
Just one sticking point, otherwise this LGTM.
iamjoemccormick
approved these changes
Jul 1, 2026
iamjoemccormick
left a comment
iamjoemccormick
left a comment
Member
There was a problem hiding this comment.
LGTM as long as the one open question isn't a valid concern.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
What does this PR do / why do we need it?
Owner/group name resolution moves into the GUFI query, plus a set of path-handling, formatting, and config-validation fixes for the
beegfs indexsubcommands.uid/gid resolution in SQL: ls/find/stat now select uidtouser(uid)/ gidtogroup(gid) in their output templates instead of resolving in Go via os/user. The CGO-disabled binary's os/user only reads /etc/passwd and /etc/group, so NSS-backed (LDAP/SSSD) names never resolved and showed as numbers (e.g. gid 5900 instead of "thinparq"). The GUFI helpers run in the index process (glibc -> NSS), so names resolve on the index host, matching how a remote index already resolves them. The Go lookupUID/ lookupGID helpers are removed. Aggregate INSERT sources stay numeric; FindAggGSelect resolves at output.
stat formatting: mode is rendered octal/symbolic (0644/-rw-r--r--) and uid/gid as num/name (1000/alice), matching gufi_stat. The resolved name columns are selected next to uid/gid.
default path for stats/info/query: an omitted path now defaults to "." (which resolves to the mount root / whole index) instead of the cwd. The cwd default failed with "not under index root" when run outside the mount, notably for a remote index where the mount-anchoring step is skipped. This matches ls/find/stat.
ls : a file argument is now listed from its parent directory with an exact name match (new LsTargetIsFile/LsFile), like ls(1)/gufi_ls, instead of returning nothing. File-vs-dir is detected locally with a cheap stat and, for a remote index, by probing the path's own summary row over gufi_query. The cmd also checks the parent index dir so a file path no longer trips "no index found".
stats median: median-leaf-* reuse GUFI's median() aggregate instead of a hand-rolled AVG over ORDER BY/LIMIT/OFFSET.
.beegfs.index validation: a malformed or unreadable file is now a hard error, raised once in the index command's PersistentPreRunE, naming the expected single [index] table. Previously it was logged at Warn and silently ignored, then surfaced confusingly later as "index root not set". loadDotIndex/DotIndexPath return the error; ApplyDotIndexOverrides leaves cfg unchanged on a bad file (the PreRun check has already reported it).
ls --skip: clarify the flag help (FILE lists directory basenames to prune, not a path to skip)
Related Issue(s)
Required when applicable.
Where should the reviewer(s) start reviewing this?
Only required for larger PRs when this may not be immediately obvious.
Are there any specific topics we should discuss before merging?
Not required.
What are the next steps after this PR?
Not required.
Checklist before merging:
Required for all PRs.
When creating a PR these are items to keep in mind that cannot be checked by GitHub actions:
For more details refer to the Go coding standards and the pull request process.