-
Notifications
You must be signed in to change notification settings - Fork 965
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
Prefetch next inode number #5130
Conversation
Signed-off-by: Changxin Miao <[email protected]>
pkg/meta/base.go
Outdated
} | ||
n := m.freeInodes.next | ||
m.freeInodes.next++ | ||
for n <= 1 { | ||
n = m.freeInodes.next | ||
m.freeInodes.next++ | ||
} | ||
if m.freeInodes.maxid-m.freeInodes.next < uint64(utils.JitterIt(inodeBatch*0.1)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it will start single goroutine if we use a fixed number here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A goroutine will be started when freeInodes.next
is close to freeInodes.maxid
, regardless of the threshold.
With a fixed number, when one inode-allocation spawns a goroutine, the next call will always spawn another one. Because the next call will have a larger freeInodes.next
.
Those goroutines except the first one are unschedulable, they will not consume much resources.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if m.freeInodes.maxid-m.freeInodes.next == fixed_one {
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, done
pkg/meta/base.go
Outdated
if err != nil { | ||
return 0, err | ||
m.prefetchMu.Lock() // Wait until prefetchInodes() is done | ||
nextLimit := m.prefetchedInode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use m.prefetchedInode to overwrite m.freeInodes if it's valid
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is what the following code does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if m.freeInodes.next >= m.freeInodes.maxid {
m.prefetchMu.Lock()
m.freeInodes = m.prefetchedInodes
m.prefetchedInodes = freeID{}
m.prefetchMu.Unlock()
}
if m.freeInodes.next >= m.freeInodes.maxid {
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactored like this?6043c1c
Signed-off-by: Changxin Miao <[email protected]>
Signed-off-by: Changxin Miao <[email protected]>
LGTM, thanks! |
Currently inodes are fetched in synchronous batches. The synchronous call to allocate inodes will harm performance sometime. One case is to write checkpoints in LLM training where hundred of nodes write at the same time - they all act synchronously which causes serious transaction conflicts. We observed hundred of milliseconds of latency when allocating inodes. One recent example:
By pipelining inode fetch and allocation, we now have a smooth latency of
create
op, e.g. (this example sets inodeBatch=10 so it fetches every 10 allocations)before:
after: