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

Add tracing and Log with usec timestamps #57

Open
wants to merge 3 commits into
base: development
Choose a base branch
from

Conversation

charmster
Copy link
Contributor

This branch is mis-named since it doesn't fix any bugs in the sortedmap code, though that was its genesis.
Add tracing messages to parts of the swiftclient code and the file system code that updates the object map (extent map) and flushes both inodes and B+ trees.

craig harmer added 3 commits December 4, 2017 12:39
Print time stamps in the log with usec precision.

Fix swiftclient tests to work with new timestamp.
Add calls to logger.Tracef() to help track down file system
corruption, timing problems, and slow performance.

Update comments for TraceLevelLogging and DebugLevelLogging
in sundry config files.
# Enable debug logging on a per-package basis. Debug logs are disabled by default unless enabled here.
# Supported values: any combination of "ldlm", "fs", "jrpcfs" and "inode" or "none" (default).
DebugLevelLogging: none

Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems to me this list is very dated... shouldn't we update it?

@@ -4,7 +4,8 @@ LogFilePath: proxyfsd.log
# NOTE: Log levels other than Trace and Debug are always on.

# Enable trace logging on a per-package basis. Trace logs are disabled by default unless enabled here.
# Supported values: jrpcfs, inode, none (default).
# Supported values: any combination of "dlm", "fs", "fuse", "headhunter", "inode", "jrpcfs", "logger",
# and "swiftclient" or "none" (default).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same comment as above... the list of packages may have grown since this was put together...

@@ -4,7 +4,8 @@ LogFilePath: /var/log/proxyfsd/proxyfsd.log
# NOTE: Log levels other than Trace and Debug are always on.

# Enable trace logging on a per-package basis. Trace logs are disabled by default unless enabled here.
# Supported values: jrpcfs, inode, none (default).
# Supported values: any combination of "dlm", "fs", "fuse", "headhunter", "inode", "jrpcfs", "logger",
# and "swiftclient" or "none" (default).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Again...

@edmc-ss
Copy link
Collaborator

edmc-ss commented Dec 8, 2017

In general, I am no fan of "let's just log/trace everything"... but can be comfortable with it IF-AND-ONLY-IF the tracing is low overhead (particularly when disabled). The current implementation is not alas. So I'm quite nervous about merging this in...

@bschatz-swift bschatz-swift self-requested a review December 8, 2017 21:53
@@ -617,6 +657,7 @@ func (vS *volumeStruct) Write(fileInodeNumber InodeNumber, offset uint64, buf []
fileInode.AttrChangeTime = updateTime
fileInode.ModificationTime = updateTime
fileInode.NumWrites++
fileInode.dirty = true
Copy link
Contributor

Choose a reason for hiding this comment

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

I am curious - did you have a specific test which was failing that caused you to find this bug?
Or, simply by reading the code did you notice this was not set correctly?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh - I see you just moved it down further with the rest of the changes.


logger.Tracef("fs.inFlightFileInodeDataTracker(): waiting to flush volume '%s' inode %d",
logger.Tracef("fs.inFlightFileInodeDataTracker() entry: waiting to flush volume '%s' inode %d",
Copy link
Contributor

Choose a reason for hiding this comment

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

Logger already adds the function name to the log message so you should not have to add the function name here.

Copy link
Contributor

Choose a reason for hiding this comment

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

For example, here is an example log message that I see when I am using this PR:

time="2017-12-11T21:00:11.737919Z" level=info msg="vs.Write(): growing file size 999424 to 1000000 volume 'CommonVolume' inode 204 offset 999424 len 576" function=Write goroutine=981 package=inode

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