Skip to content

Commit

Permalink
merge to stable-23-3: some important fixes and TODO implementations f…
Browse files Browse the repository at this point in the history
…or multitablet filesystems + some minor fixes/enhancements in filestore in general (#1523)

* [Filestore] Add stat command to filestore client (#1453)

Issue: #1474

* fixed filestore-client crash upon empty dir ls, added ls+rm+write test for filestore-client (#1505)

* fixed filestore-client crash upon empty dir ls, added ls+rm+write test for filestore-client

* fixed filestore-client crash upon empty dir ls, added ls+rm+write test for filestore-client
- forgot client.py

* issue-1350: simplified node creation in follower code, added a test for errors received from follower in this case, fixed DupCache entry commitment in CreateHandle, made TabletProxy immortal, deleted inactive pipe tracking and closure in TabletProxy, added some comments, fixed logging a bit (#1511)

* issue-1350: simplified node creation in follower code, added a test for errors received from follower in this case, added some comments, fixed logging a bit

* issue-1350: CreateHandle - if we need to create a node in one of the followers, we still need to commit DupCache entry;

* issue-1350: TabletProxy should never die, TabletProxy should not track and close idle connections - 1. there is no need for that 2. it breaks leader<->follower session creation logic

* filestore loop shutdown: 1. E_CANCELLED code shouldn't be wrapped into MAKE_FILESTORE_ERROR 2. CompletionQueue should be notified about request cancellation (#1520)

* filestore loop shutdown: 1. E_CANCELLED code shouldn't be wrapped into MAKE_FILESTORE_ERROR 2. CompletionQueue should be notified about request cancellation

* filestore loop shutdown: 1. E_CANCELLED code shouldn't be wrapped into MAKE_FILESTORE_ERROR 2. CompletionQueue should be notified about request cancellation - added missing include

* filestore loop shutdown: 1. E_CANCELLED code shouldn't be wrapped into MAKE_FILESTORE_ERROR 2. CompletionQueue should be notified about request cancellation - added missing include

* filestore loop shutdown: 1. E_CANCELLED code shouldn't be wrapped into MAKE_FILESTORE_ERROR 2. CompletionQueue should be notified about request cancellation - fixed build

* issue-1350: implementing multitablet fs TODOs - DestroySession in followers, UnlinkNode (#1521)

* issue-1350: destroying sessions in follower upon session destruction in leader

* issue-1350: removing node refs in leader and unlinking nodes in followers upon UnlinkNode requests to leader

* issue-1350: 1. Cleanup is given priority over Compaction by default since it's much faster and lighter but generally brings similar results (configurable via StorageServiceConfig) 2. Outputting follower info on monpage (#1522)

* issue-1350: 1. Cleanup is given priority over Compaction by default since it's much faster and lighter but generally brings similar results (configurable via StorageServiceConfig) 2. Outputting follower info on monpage

* issue-1350: deleted unneeded include

* fixed CMakeLists.txt

---------

Co-authored-by: Anton Myagkov <antomyagkov@gmail.com>
  • Loading branch information
qkrorlqr and antonmyagkov committed Jun 30, 2024
1 parent e6549f6 commit da542e4
Show file tree
Hide file tree
Showing 37 changed files with 965 additions and 151 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ target_sources(filestore-apps-client-lib PRIVATE
${CMAKE_SOURCE_DIR}/cloud/filestore/apps/client/lib/resize.cpp
${CMAKE_SOURCE_DIR}/cloud/filestore/apps/client/lib/rm.cpp
${CMAKE_SOURCE_DIR}/cloud/filestore/apps/client/lib/start_endpoint.cpp
${CMAKE_SOURCE_DIR}/cloud/filestore/apps/client/lib/stat.cpp
${CMAKE_SOURCE_DIR}/cloud/filestore/apps/client/lib/stop_endpoint.cpp
${CMAKE_SOURCE_DIR}/cloud/filestore/apps/client/lib/text_table.cpp
${CMAKE_SOURCE_DIR}/cloud/filestore/apps/client/lib/touch.cpp
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ target_sources(filestore-apps-client-lib PRIVATE
${CMAKE_SOURCE_DIR}/cloud/filestore/apps/client/lib/resize.cpp
${CMAKE_SOURCE_DIR}/cloud/filestore/apps/client/lib/rm.cpp
${CMAKE_SOURCE_DIR}/cloud/filestore/apps/client/lib/start_endpoint.cpp
${CMAKE_SOURCE_DIR}/cloud/filestore/apps/client/lib/stat.cpp
${CMAKE_SOURCE_DIR}/cloud/filestore/apps/client/lib/stop_endpoint.cpp
${CMAKE_SOURCE_DIR}/cloud/filestore/apps/client/lib/text_table.cpp
${CMAKE_SOURCE_DIR}/cloud/filestore/apps/client/lib/touch.cpp
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ target_sources(filestore-apps-client-lib PRIVATE
${CMAKE_SOURCE_DIR}/cloud/filestore/apps/client/lib/resize.cpp
${CMAKE_SOURCE_DIR}/cloud/filestore/apps/client/lib/rm.cpp
${CMAKE_SOURCE_DIR}/cloud/filestore/apps/client/lib/start_endpoint.cpp
${CMAKE_SOURCE_DIR}/cloud/filestore/apps/client/lib/stat.cpp
${CMAKE_SOURCE_DIR}/cloud/filestore/apps/client/lib/stop_endpoint.cpp
${CMAKE_SOURCE_DIR}/cloud/filestore/apps/client/lib/text_table.cpp
${CMAKE_SOURCE_DIR}/cloud/filestore/apps/client/lib/touch.cpp
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ target_sources(filestore-apps-client-lib PRIVATE
${CMAKE_SOURCE_DIR}/cloud/filestore/apps/client/lib/resize.cpp
${CMAKE_SOURCE_DIR}/cloud/filestore/apps/client/lib/rm.cpp
${CMAKE_SOURCE_DIR}/cloud/filestore/apps/client/lib/start_endpoint.cpp
${CMAKE_SOURCE_DIR}/cloud/filestore/apps/client/lib/stat.cpp
${CMAKE_SOURCE_DIR}/cloud/filestore/apps/client/lib/stop_endpoint.cpp
${CMAKE_SOURCE_DIR}/cloud/filestore/apps/client/lib/text_table.cpp
${CMAKE_SOURCE_DIR}/cloud/filestore/apps/client/lib/touch.cpp
Expand Down
2 changes: 2 additions & 0 deletions cloud/filestore/apps/client/lib/factory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ TCommandPtr NewWriteCommand();
TCommandPtr NewExecuteActionCommand();
TCommandPtr NewCreateSessionCommand();
TCommandPtr NewResetSessionCommand();
TCommandPtr NewStatCommand();

////////////////////////////////////////////////////////////////////////////////

Expand Down Expand Up @@ -60,6 +61,7 @@ static const TMap<TString, TFactoryFunc> Commands = {
{ "executeaction", NewExecuteActionCommand },
{ "createsession", NewCreateSessionCommand },
{ "resetsession", NewResetSessionCommand },
{ "stat", NewStatCommand },
};

////////////////////////////////////////////////////////////////////////////////
Expand Down
68 changes: 68 additions & 0 deletions cloud/filestore/apps/client/lib/stat.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
#include "command.h"

#include <cloud/filestore/public/api/protos/fs.pb.h>

#include <util/datetime/base.h>
#include <util/stream/file.h>
#include <util/system/sysstat.h>

namespace NCloud::NFileStore::NClient {

namespace {

////////////////////////////////////////////////////////////////////////////////

void Print(const NProto::TNodeAttr& nodeAttr, bool jsonOutput) {

if (jsonOutput) {
Cout << nodeAttr.AsJSON() << Endl;
} else {
Cout << nodeAttr.DebugString() << Endl;
}
}

////////////////////////////////////////////////////////////////////////////////

class TStatCommand final: public TFileStoreCommand
{
private:
TString Path;

public:
TStatCommand()
{
Opts.AddLongOption("path")
.Required()
.RequiredArgument("PATH")
.StoreResult(&Path);
}

bool Execute() override
{
CreateSession();

const auto resolved = ResolvePath(Path, false);
Y_ABORT_UNLESS(resolved.size() >= 2);

auto request = CreateRequest<NProto::TGetNodeAttrRequest>();
request->SetNodeId(resolved[resolved.size() - 2].Node.GetId());
request->SetName(ToString(resolved.back().Name));
auto response = WaitFor(
Client->GetNodeAttr(PrepareCallContext(), std::move(request)));

CheckResponse(response);
Print(response.GetNode(), JsonOutput);
return true;
}
};

} // namespace

////////////////////////////////////////////////////////////////////////////////

TCommandPtr NewStatCommand()
{
return std::make_shared<TStatCommand>();
}

} // namespace NCloud::NFileStore::NClient
4 changes: 4 additions & 0 deletions cloud/filestore/apps/client/lib/text_table.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ void PrintRows(IOutputStream& out, const TTextTable& table)
out << row.back();
};
const auto& rows = table.GetRows();
if (rows.empty()) {
return;
}

for (size_t i = 0; i < rows.size() - 1; ++i) {
printRow(rows[i]);
out << Endl;
Expand Down
1 change: 1 addition & 0 deletions cloud/filestore/apps/client/lib/ya.make
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ SRCS(
resize.cpp
rm.cpp
start_endpoint.cpp
stat.cpp
stop_endpoint.cpp
text_table.cpp
touch.cpp
Expand Down
12 changes: 12 additions & 0 deletions cloud/filestore/config/storage.proto
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,15 @@ option go_package = "a.yandex-team.ru/cloud/filestore/config";

////////////////////////////////////////////////////////////////////////////////

enum EBlobIndexOpsPriority
{
BIOP_CLEANUP_FIRST = 0;
BIOP_COMPACTION_FIRST = 1;
BIOP_FAIR = 2;
}

////////////////////////////////////////////////////////////////////////////////

message TStorageConfig
{
// Schemeshard directory for tablets.
Expand Down Expand Up @@ -286,4 +295,7 @@ message TStorageConfig
// Forwarding is done based on the NodeId and Handle high bits.
// Disabling this thing shouldn't really be needed apart from tests.
optional bool MultiTabletForwardingEnabled = 355;

// Controls BlobIndexOps' priority over each other.
optional EBlobIndexOpsPriority BlobIndexOpsPriority = 356;
}
10 changes: 10 additions & 0 deletions cloud/filestore/libs/storage/core/config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ namespace {
xxx(MaxBlocksPerTruncateTx, ui32, 0 /*TODO: 32GiB/4KiB*/ )\
xxx(MaxTruncateTxInflight, ui32, 10 )\
xxx(CompactionRetryTimeout, TDuration, TDuration::Seconds(1) )\
xxx(BlobIndexOpsPriority, \
NProto::EBlobIndexOpsPriority, \
NProto::BIOP_CLEANUP_FIRST )\
\
xxx(FlushThresholdForBackpressure, ui32, 128_MB )\
xxx(CleanupThresholdForBackpressure, ui32, 32768 )\
Expand Down Expand Up @@ -197,6 +200,13 @@ IOutputStream& operator <<(
return out << EAuthorizationMode_Name(mode);
}

IOutputStream& operator <<(
IOutputStream& out,
NProto::EBlobIndexOpsPriority biopp)
{
return out << EBlobIndexOpsPriority_Name(biopp);
}

template <typename T>
void DumpImpl(const T& t, IOutputStream& os)
{
Expand Down
2 changes: 2 additions & 0 deletions cloud/filestore/libs/storage/core/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,8 @@ class TStorageConfig

bool GetMultiTabletForwardingEnabled() const;

NProto::EBlobIndexOpsPriority GetBlobIndexOpsPriority() const;

void Dump(IOutputStream& out) const;
void DumpHtml(IOutputStream& out) const;
void DumpOverridesHtml(IOutputStream& out) const;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ void TStorageServiceActor::HandleCreateHandle(
ctx,
TFileStoreComponents::SERVICE,
"create handle in follower %s",
msg->Record.DebugString().c_str());
msg->Record.ShortDebugString().Quote().c_str());

auto [cookie, inflight] = CreateInFlightRequest(
TRequestInfo(ev->Sender, ev->Cookie, msg->CallContext),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ void TListNodesActor::HandleGetNodeAttrResponse(
return;
}

LOG_INFO(
LOG_DEBUG(
ctx,
TFileStoreComponents::SERVICE,
"GetNodeAttrResponse from follower: %s",
Expand Down
155 changes: 155 additions & 0 deletions cloud/filestore/libs/storage/service/service_ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2659,6 +2659,45 @@ Y_UNIT_TEST_SUITE(TStorageServiceTest)
handle2,
0,
TString(1_MB, 'a'));

for (const auto& shardId: {shard1Id, shard2Id}) {
NProtoPrivate::TDescribeSessionsRequest request;
request.SetFileSystemId(shardId);

TString buf;
google::protobuf::util::MessageToJsonString(request, &buf);
auto jsonResponse = service.ExecuteAction("describesessions", buf);
NProtoPrivate::TDescribeSessionsResponse response;
UNIT_ASSERT(google::protobuf::util::JsonStringToMessage(
jsonResponse->Record.GetOutput(), &response).ok());

const auto& sessions = response.GetSessions();
UNIT_ASSERT_VALUES_EQUAL(1, sessions.size());

UNIT_ASSERT_VALUES_EQUAL(
headers.SessionId,
sessions[0].GetSessionId());
UNIT_ASSERT_VALUES_EQUAL(
headers.ClientId,
sessions[0].GetClientId());
}

service.DestroySession(headers);

for (const auto& shardId: {shard1Id, shard2Id}) {
NProtoPrivate::TDescribeSessionsRequest request;
request.SetFileSystemId(shardId);

TString buf;
google::protobuf::util::MessageToJsonString(request, &buf);
auto jsonResponse = service.ExecuteAction("describesessions", buf);
NProtoPrivate::TDescribeSessionsResponse response;
UNIT_ASSERT(google::protobuf::util::JsonStringToMessage(
jsonResponse->Record.GetOutput(), &response).ok());

const auto& sessions = response.GetSessions();
UNIT_ASSERT_VALUES_EQUAL(0, sessions.size());
}
}

Y_UNIT_TEST(ShouldCreateNodeInFollowerViaLeader)
Expand Down Expand Up @@ -2996,6 +3035,46 @@ Y_UNIT_TEST_SUITE(TStorageServiceTest)
0,
listNodesResponse.GetNodes(1).GetSize());

service.UnlinkNode(headers, RootNodeId, "file1");

listNodesResponse = service.ListNodes(
headers,
fsId,
RootNodeId)->Record;

UNIT_ASSERT_VALUES_EQUAL(1, listNodesResponse.NamesSize());
UNIT_ASSERT_VALUES_EQUAL("file2", listNodesResponse.GetNames(0));

UNIT_ASSERT_VALUES_EQUAL(1, listNodesResponse.NodesSize());
UNIT_ASSERT_VALUES_EQUAL(
nodeId2,
listNodesResponse.GetNodes(0).GetId());
UNIT_ASSERT_VALUES_EQUAL(
0,
listNodesResponse.GetNodes(0).GetSize());

auto headers1 = headers;
headers1.FileSystemId = shard1Id;

listNodesResponse = service.ListNodes(
headers1,
shard1Id,
RootNodeId)->Record;

UNIT_ASSERT_VALUES_EQUAL(0, listNodesResponse.NamesSize());
UNIT_ASSERT_VALUES_EQUAL(0, listNodesResponse.NodesSize());

auto headers2 = headers;
headers2.FileSystemId = shard2Id;

listNodesResponse = service.ListNodes(
headers2,
shard2Id,
RootNodeId)->Record;

UNIT_ASSERT_VALUES_EQUAL(1, listNodesResponse.NamesSize());
UNIT_ASSERT_VALUES_EQUAL(1, listNodesResponse.NodesSize());

// TODO(#1350): test XAttr requests
}

Expand Down Expand Up @@ -3145,6 +3224,82 @@ Y_UNIT_TEST_SUITE(TStorageServiceTest)
data.Size())->Record;
UNIT_ASSERT_VALUES_EQUAL(data, readDataResponse.GetBuffer());
}

Y_UNIT_TEST(ShouldHandleCreateNodeErrorFromFollowerUponCreateHandleViaLeader)
{
NProto::TStorageConfig config;
config.SetMultiTabletForwardingEnabled(true);
TTestEnv env({}, config);
env.CreateSubDomain("nfs");

ui32 nodeIdx = env.CreateNode("nfs");

const TString fsId = "test";
const auto shard1Id = fsId + "-f1";
const auto shard2Id = fsId + "-f2";

TServiceClient service(env.GetRuntime(), nodeIdx);
service.CreateFileStore(fsId, 1'000);
service.CreateFileStore(shard1Id, 1'000);
service.CreateFileStore(shard2Id, 1'000);

ConfigureFollowers(service, fsId, shard1Id, shard2Id);

auto headers = service.InitSession(fsId, "client");

auto error = MakeError(E_FS_INVALID_SESSION, "bad session");

env.GetRuntime().SetEventFilter(
[&] (TTestActorRuntimeBase&, TAutoPtr<IEventHandle>& event) {
switch (event->GetTypeRewrite()) {
case TEvService::EvCreateNodeResponse: {
auto* msg =
event->Get<TEvService::TEvCreateNodeResponse>();
if (error.GetCode()) {
msg->Record.MutableError()->CopyFrom(error);
}

break;
}
}

return false;
});

const ui64 requestId = 111;

service.SendCreateHandleRequest(
headers,
fsId,
RootNodeId,
"file1",
TCreateHandleArgs::CREATE_EXL,
"",
requestId);

auto response = service.RecvCreateHandleResponse();
UNIT_ASSERT_VALUES_EQUAL_C(
error.GetCode(),
response->GetError().GetCode(),
FormatError(response->GetError()));

error = {};

service.SendCreateHandleRequest(
headers,
fsId,
RootNodeId,
"file1",
TCreateHandleArgs::CREATE_EXL,
"",
requestId);

response = service.RecvCreateHandleResponse();
UNIT_ASSERT_VALUES_EQUAL_C(
error.GetCode(),
response->GetError().GetCode(),
FormatError(response->GetError()));
}
}

} // namespace NCloud::NFileStore::NStorage
Loading

0 comments on commit da542e4

Please sign in to comment.