-
Notifications
You must be signed in to change notification settings - Fork 39
feat: add support for multi-stream SSDs (such as FDP SSDs) #92
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ | |
| #include <toolkit/BitStore.h> | ||
| #include <app/App.h> | ||
|
|
||
| #define RW_HINT_INVALID 0xFF | ||
|
|
||
| struct RemotingIOInfo; | ||
| typedef struct RemotingIOInfo RemotingIOInfo; | ||
|
|
@@ -44,6 +45,7 @@ struct RemotingIOInfo | |
| #ifdef BEEGFS_NVFS | ||
| bool nvfs; | ||
| #endif | ||
| uint64_t writeHint; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. todo: we need to ensure the |
||
| }; | ||
|
|
||
|
|
||
|
|
@@ -78,6 +80,8 @@ void RemotingIOInfo_initOpen(App* app, unsigned accessFlags, AtomicInt* maxUsedT | |
| #ifdef BEEGFS_NVFS | ||
| outIOInfo->nvfs = false; | ||
| #endif | ||
|
|
||
| outIOInfo->writeHint = RW_HINT_INVALID; | ||
| } | ||
|
|
||
|
|
||
|
|
@@ -104,6 +108,8 @@ void RemotingIOInfo_initSpecialClose(App* app, const char* fileHandleID, | |
| #ifdef BEEGFS_NVFS | ||
| outIOInfo->nvfs = false; | ||
| #endif | ||
|
|
||
| outIOInfo->writeHint = RW_HINT_INVALID; | ||
| } | ||
|
|
||
| /** | ||
|
|
||
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.
todo: Unfortunately adding fields to network messages like the
writeHinttoWriteLocalFileMsgandWriteLocalFileRDMAMsgwill create incompatibilities when there are mixed client/server versions. If a client sends a message with this field to a server that doesn't know about it yet, the server-side message deserialization will fail. This is also a problem if the server was updated and expects the new message format, but the client omits this field.The expectation is that any 8.x client can communicate to any 8.x server. While sometimes we can find a way to rollout a change like this in a minor BeeGFS release, it will always require extra handling. Because this tends to introduce technical debt we have to go back and cleanup at the next major release, we try to avoid these kinds of changes if possible.
So I'll need to still justify the change internally. It would be helpful if you could share any testing you've done that demonstrates the performance improvement and/or other benefits you've seen with this patch?
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.
Sure. I've added more details to the pull request. Please take a look.