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(multi-tenancy): Format namespace to human readable form #7552

Merged
merged 3 commits into from
Mar 15, 2021

Conversation

ahsanbarkati
Copy link
Contributor

@ahsanbarkati ahsanbarkati commented Mar 11, 2021

This PR changes the logs to print namespace in human-readable form. This change doesn't fix all of them.
We have left the cases where we print a struct and the fields in the struct contains predicate with namespace.
We have also skipped logs of SubscribeForUpdates because we can't assume the prefix being logged will always contain namespace.

This is not exhaustive, it fixes the things which are apparent on basic usage of dgraph.


This change is Reviewable

Copy link
Contributor

@jarifibrahim jarifibrahim left a comment

Choose a reason for hiding this comment

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

Print the namespace information also.

@@ -592,15 +592,16 @@ func (r *rebuilder) Run(ctx context.Context) error {

glog.V(1).Infof(
"Rebuilding index for predicate %s: Starting process. StartTs=%d. Prefix=\n%s\n",
r.attr, r.startTs, hex.Dump(r.prefix))
x.FormatNsAttr(r.attr), r.startTs, hex.Dump(r.prefix))
Copy link
Contributor

Choose a reason for hiding this comment

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

Print namespace as well.


// Counter is used here to ensure that all keys are committed at different timestamp.
// We set it to 1 in case there are no keys found and NewStreamAt is called with ts=0.
var counter uint64 = 1

tmpWriter := tmpDB.NewManagedWriteBatch()
stream := pstore.NewStreamAt(r.startTs)
stream.LogPrefix = fmt.Sprintf("Rebuilding index for predicate %s (1/2):", r.attr)
stream.LogPrefix = fmt.Sprintf("Rebuilding index for predicate %s (1/2):",
Copy link
Contributor

Choose a reason for hiding this comment

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

Print namespace and attr

@@ -662,19 +663,21 @@ func (r *rebuilder) Run(ctx context.Context) error {
return err
}
glog.V(1).Infof("Rebuilding index for predicate %s: building temp index took: %v\n",
r.attr, time.Since(start))
x.FormatNsAttr(r.attr), time.Since(start))
Copy link
Contributor

Choose a reason for hiding this comment

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

here as well. Print namespace everywhere. Without the namespace, we wouldn't know which namespace is doing this operation.

Copy link
Contributor Author

@ahsanbarkati ahsanbarkati Mar 13, 2021

Choose a reason for hiding this comment

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

It is already printing namespace information.
FormatNsAttr(attr) formats them to this format: x-predicate where x is the namespace.

predicate name from namespace 1 will be printed as 1-name rather than current thing which is /000/000/000/000/000/000/000/001name

Copy link
Contributor

Choose a reason for hiding this comment

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

My bad. I didn't realize the formatNSAttr was adding the namespace as well.

@ahsanbarkati ahsanbarkati merged commit 42a9f6f into release/v21.03 Mar 15, 2021
@ahsanbarkati ahsanbarkati deleted the ahsan/multi-tenancy-logs branch March 15, 2021 07:42
aman-bansal pushed a commit that referenced this pull request Mar 16, 2021
Change the logs to print namespace in human-readable form.
It also fixes the state endpoint to print predicates cleanly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants