-
Notifications
You must be signed in to change notification settings - Fork 38
issue-1751: [Filestore] Restore and drain WriteBackCache at session recreation #4711
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: main
Are you sure you want to change the base?
Changes from all commits
507f7d3
7872877
5dc74cd
cbc2990
5455fea
ac1e021
678cb02
8bc22e3
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 |
|---|---|---|
|
|
@@ -412,48 +412,74 @@ void TFileSystem::Write( | |
| request->SetBufferOffset(alignedBuffer.AlignedDataOffset()); | ||
| request->SetBuffer(alignedBuffer.TakeBuffer()); | ||
|
|
||
| const auto size = buffer.size(); | ||
| DoWrite(callContext, req, ino, std::move(request), buffer.size(), fi); | ||
| } | ||
|
|
||
| void TFileSystem::DoWrite( | ||
| TCallContextPtr callContext, | ||
| fuse_req_t req, | ||
| fuse_ino_t ino, | ||
| std::shared_ptr<NProto::TWriteDataRequest> request, | ||
| ui64 size, | ||
| fuse_file_info* fi) | ||
| { | ||
| const auto wbcMode = GetServerWriteBackCacheState(fi); | ||
|
|
||
| const auto handle = fi->fh; | ||
| const auto reqId = callContext->RequestId; | ||
|
|
||
| auto callback = [=, ptr = weak_from_this()](const auto& future) | ||
| { | ||
| auto self = ptr.lock(); | ||
| if (!self) { | ||
| return; | ||
| } | ||
|
|
||
| const auto& response = future.GetValue(); | ||
| const auto& error = response.GetError(); | ||
|
|
||
| if (ShouldUseServerWriteBackCache(fi)) { | ||
| if (wbcMode != EServerWriteBackCacheState::Enabled) { | ||
| self->FSyncQueue | ||
| ->Dequeue(reqId, error, TNodeId{ino}, THandle{handle}); | ||
| } | ||
|
|
||
| if (self->CheckResponse(self, *callContext, req, response)) { | ||
| self->ReplyWrite(*callContext, error, req, size); | ||
| } | ||
| }; | ||
|
|
||
| if (wbcMode == EServerWriteBackCacheState::Enabled) { | ||
| WriteBackCache.WriteData(callContext, std::move(request)) | ||
| .Subscribe( | ||
| [=, | ||
| ptr = weak_from_this()] (const auto& future) | ||
| { | ||
| auto self = ptr.lock(); | ||
| if (!self) { | ||
| return; | ||
| } | ||
|
|
||
| const auto& response = future.GetValue(); | ||
| const auto& error = response.GetError(); | ||
|
|
||
| if (CheckResponse(self, *callContext, req, response)) { | ||
| self->ReplyWrite(*callContext, error, req, size); | ||
| } | ||
| }); | ||
| .Subscribe(std::move(callback)); | ||
| return; | ||
| } | ||
|
|
||
| const auto handle = fi->fh; | ||
| const auto reqId = callContext->RequestId; | ||
| FSyncQueue->Enqueue(reqId, TNodeId {ino}, THandle {handle}); | ||
|
|
||
| Session->WriteData(callContext, std::move(request)) | ||
| .Subscribe([=, ptr = weak_from_this()] (const auto& future) { | ||
| auto self = ptr.lock(); | ||
| if (!self) { | ||
| return; | ||
| } | ||
| if (wbcMode == EServerWriteBackCacheState::Disabled) { | ||
| Session->WriteData(callContext, std::move(request)) | ||
| .Subscribe(std::move(callback)); | ||
| return; | ||
| } | ||
|
|
||
| const auto& response = future.GetValue(); | ||
| const auto& error = response.GetError(); | ||
| self->FSyncQueue->Dequeue(reqId, error, TNodeId {ino}, THandle {handle}); | ||
| Y_ABORT_UNLESS( | ||
|
Collaborator
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. let's use STORAGE_VERIFY / STORAGE_VERIFY_C macros - they force the developer to provide the entity id (filesystem id / tablet id / client id / etc), otherwise we'll just see that some error happened without knowing anything about the entity that caused the problem (so the debugging process will take more time) here we can use client id in the STORAGE_VERIFY message
Collaborator
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. Can be done in a separate PR - there're many other ABORT_UNLESS usages in vfs_fuse which would be nice to replace with STORAGE_VERIFY(clientId)
Collaborator
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. I would say, that ABORT_UNLESS is the only usage for checks in vfs_fuse and filestore vhost for now. Wasn't able to find a single instance of Verify in those components |
||
| wbcMode == EServerWriteBackCacheState::Draining, | ||
| "Invalid EServerWriteBackCacheState value = %d", | ||
| wbcMode); | ||
|
|
||
| if (CheckResponse(self, *callContext, req, response)) { | ||
| self->ReplyWrite(*callContext, error, req, size); | ||
| } | ||
| }); | ||
| WriteBackCache.FlushNodeData(request->GetNodeId()) | ||
| .Subscribe( | ||
| [ptr = weak_from_this(), | ||
| callback = std::move(callback), | ||
| callContext = std::move(callContext), | ||
| request = std::move(request)](const auto& f) mutable | ||
| { | ||
| f.GetValue(); | ||
| if (auto self = ptr.lock()) { | ||
| self->Session->WriteData(callContext, std::move(request)) | ||
| .Subscribe(std::move(callback)); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| void TFileSystem::WriteBufLocal( | ||
|
|
@@ -577,45 +603,7 @@ void TFileSystem::WriteBuf( | |
| request->SetHandle(fi->fh); | ||
| request->SetOffset(offset); | ||
|
|
||
| if (ShouldUseServerWriteBackCache(fi)) { | ||
| WriteBackCache.WriteData(callContext, std::move(request)) | ||
| .Subscribe( | ||
| [=, ptr = weak_from_this()] (const auto& future) | ||
| { | ||
| auto self = ptr.lock(); | ||
| if (!self) { | ||
| return; | ||
| } | ||
|
|
||
| const auto& response = future.GetValue(); | ||
| const auto& error = response.GetError(); | ||
|
|
||
| if (CheckResponse(self, *callContext, req, response)) { | ||
| self->ReplyWrite(*callContext, error, req, size); | ||
| } | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| const auto handle = fi->fh; | ||
| const auto reqId = callContext->RequestId; | ||
| FSyncQueue->Enqueue(reqId, TNodeId {ino}, THandle {handle}); | ||
|
|
||
| Session->WriteData(callContext, std::move(request)) | ||
| .Subscribe([=, ptr = weak_from_this()] (const auto& future) { | ||
| auto self = ptr.lock(); | ||
| if (!self) { | ||
| return; | ||
| } | ||
|
|
||
| const auto& response = future.GetValue(); | ||
| const auto& error = response.GetError(); | ||
| self->FSyncQueue->Dequeue(reqId, error, TNodeId {ino}, THandle {handle}); | ||
|
|
||
| if (CheckResponse(self, *callContext, req, response)) { | ||
| self->ReplyWrite(*callContext, error, req, size); | ||
| } | ||
| }); | ||
| DoWrite(callContext, req, ino, std::move(request), size, fi); | ||
| } | ||
|
|
||
| void TFileSystem::FAllocate( | ||
|
|
@@ -960,17 +948,23 @@ void TFileSystem::FSyncDir( | |
|
|
||
| //////////////////////////////////////////////////////////////////////////////// | ||
|
|
||
| bool TFileSystem::ShouldUseServerWriteBackCache(const fuse_file_info* fi) const | ||
| EServerWriteBackCacheState TFileSystem::GetServerWriteBackCacheState( | ||
| const fuse_file_info* fi) const | ||
| { | ||
| if (!WriteBackCache || !Config->GetServerWriteBackCacheEnabled()) { | ||
| return false; | ||
| if (!WriteBackCache) { | ||
| return EServerWriteBackCacheState::Disabled; | ||
| } | ||
|
|
||
| if (fi->flags & O_DIRECT) { | ||
| return false; | ||
| return EServerWriteBackCacheState::Disabled; | ||
| } | ||
|
|
||
| return true; | ||
| if (Config->GetServerWriteBackCacheEnabled()) { | ||
| return EServerWriteBackCacheState::Enabled; | ||
| } | ||
|
|
||
| return WriteBackCache.IsEmpty() ? EServerWriteBackCacheState::Disabled | ||
| : EServerWriteBackCacheState::Draining; | ||
| } | ||
|
|
||
| } // namespace NCloud::NFileStore::NFuse | ||
Uh oh!
There was an error while loading. Please reload this page.