-
Notifications
You must be signed in to change notification settings - Fork 76
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
complete upload - skip db access for small object (for performance) #7486
Conversation
e7c12f2
to
d8f87a2
Compare
} else if (req?.params?.num_parts === 1) { | ||
//skip db access for small objects | ||
dbg.log1("skipped _complete_object_multiparts"); | ||
map_res = {size: req.rpc_params.size, num_parts: 1}; | ||
} else { | ||
map_res = await _complete_object_parts(obj); | ||
} |
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.
I would prefer to move this optimization into _complete_object_parts()
(need to send params.num_parts inside).
Over there we can start by identifying a single part case, and optimize the hell of it, so that we don't even read the part from the db, which is an aggressive optimization which we also need to explain why it is safe (the reason is that mpu cannot have a single part per multipart... which is a bit of a "bad" assumption to take because it could easily get broken one day by someone who just doesn't expect it calling these apis).
We can also add a simpler condition that will still read the parts, but will skip updating them by noticing that they are already committed and do not require an offset/seq update - simply surround the push to parts_updates with a condition like
if (part.uncommitted || updates.set_updates) {
context.parts_updates.push(updates);
}
on this line -
context.parts_updates.push(updates); |
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.
I've moved the check whether to skip the db into _complete_next_parts().
Since we're going for performance, I figured it's best to skip the db network hop altogether.
You think it's not safe enough?
9a7810c
to
23919f1
Compare
b95e540
to
d862801
Compare
d862801
to
a89da1a
Compare
a89da1a
to
604b3d4
Compare
604b3d4
to
dc62bbf
Compare
dc62bbf
to
ba67242
Compare
ba67242
to
5022402
Compare
Signed-off-by: Amit Prinz Setter <[email protected]>
Signed-off-by: Amit Prinz Setter <[email protected]>
…nstead of MapServer) Signed-off-by: Amit Prinz Setter <[email protected]>
Signed-off-by: Amit Prinz Setter <[email protected]>
Signed-off-by: Amit Prinz Setter <[email protected]>
Signed-off-by: Amit Prinz Setter <[email protected]>
5022402
to
eb39ce8
Compare
This PR had no activity for too long - it will now be labeled stale. Update it to prevent it from getting closed. |
This PR is stale and had no activity for too long - it will now be closed. |
Explain the changes
For small objects with one part, we know the start/end of object.
There's no need to fix values in db at the end of the put object.
(It's needed for multipart upload where each part doesn't know its start/end at single part upload).
Also, set part as committed when inserting the part (again, to skip db access for removing uncommitted at the end).
Issues: Fixed #xxx / Gap #xxx
Redundant db access at the end of upload.
Testing Instructions: