issue-2639: [Filestore] Read and write compressed ShardId and ShardNodeName to NodeRefs#5545
issue-2639: [Filestore] Read and write compressed ShardId and ShardNodeName to NodeRefs#5545ruslan-gar wants to merge 3 commits intomainfrom
Conversation
| 1); | ||
| 1, | ||
| Config->GetShardIdCompressionMode(), | ||
| GetFileSystemId()); |
There was a problem hiding this comment.
Are you sure that this is the same as the Main filesystem? In cases of sharded directories, a shard with the name filesystem_s12 could hold an external reference to filesystem_s34
There was a problem hiding this comment.
Yep you need GetMainFileSystemId
There was a problem hiding this comment.
also pls add a test with directory sharding
| #define lengthof(arr) sizeof(arr) / sizeof(arr[0]) | ||
|
|
||
| template<typename TNumType> | ||
| void numberFromByteStream(TNumType& num, const char*& byteStream) |
There was a problem hiding this comment.
lower case naming convention
| #define lengthof(arr) sizeof(arr) / sizeof(arr[0]) | ||
|
|
||
| template<typename TNumType> | ||
| void numberFromByteStream(TNumType& num, const char*& byteStream) |
There was a problem hiding this comment.
consider const char*& -> const char*
There was a problem hiding this comment.
Oh, let us return num in the return value?
| } | ||
| } | ||
|
|
||
| inline bool IsEncoded(const TIndexTabletDatabase::TNodeRef& nodeRef) |
There was a problem hiding this comment.
Shouldn't it be a TNodeRef Method?
| inline bool IsEncoded(const TIndexTabletDatabase::TNodeRef& nodeRef) | ||
| { | ||
| return !nodeRef.ShardId.empty() && | ||
| nodeRef.ShardId[0] == ShardIdAsBinaryStream; |
There was a problem hiding this comment.
Let us ensure human-readability here with isalnum or something like that?
And add a compile-time assert that ShardIdAsBinaryStream is not readable (in case we are going to add new formats
| public: | ||
|
|
||
| TMaxTracker(size_t slots = 15) | ||
| TMaxTracker(size_t slots = 15) |
|
|
||
| config.SetMultiTabletForwardingEnabled(true); | ||
| config.SetStrictFileSystemSizeEnforcementEnabled(true); | ||
| config.SetShardBalancerPolicy(NProto::SBP_ROUND_ROBIN); |
There was a problem hiding this comment.
it is ROUND_ROBIN by default
| config.SetAutomaticShardCreationEnabled(true); | ||
| config.SetShardAllocationUnit(shardAllocationUnit); | ||
|
|
||
| const TString fsId = "computefilesystem-e0tdfty89a5fjdpg6k"; |
There was a problem hiding this comment.
let us use something more neutral
|
|
||
| for (int writeModeNum = | ||
| NProto::EShardIdCompressionMode::SICM_NO_COMPRESSION; | ||
| writeModeNum <= |
There was a problem hiding this comment.
Is it worth to introduce ::MAX value to be able to properly iterate?
| NProto::TStorageConfig newStorageConfig; | ||
| auto setMode = [&](const NProto::EShardIdCompressionMode mode) { | ||
| newStorageConfig.SetShardIdCompressionMode(mode); | ||
| ChangeStorageConfig(fsId, newStorageConfig, service, true); |
There was a problem hiding this comment.
If you are going to move this test to the one for directory sharding, do not forget to apply it to all shards
But probably it is easier to change the runtime storage config and restart the tablet
There was a problem hiding this comment.
I am not sure it will be applied to the restarted tablets, though
Notes
ShardId and ShardNodeName can be read and written to NodeRefs table in a compressed format.
SjhardId now takes 3 bytes: 1 - compression type, 2 bytes for shardNo.
SharNodeName takes 16 bytes: it's just a GUID in the form of four 4-byte integers.
Both fields are written to the same columns as before.
In order to get the same ShardId as before, a filesystem id is passed the functions that read NodeRefs
How ShardId/ShardNodeName are written and read is controlled by:
enum EShardIdCompressionMode
{
// Only uncompressed ShartdId/ShardNodeName are read and written.
SICM_NO_COMPRESSION = 0;
// Both uncompressed and compressed forms are read.
SICM_READ = 1;
// SICM_READ + new ShartdId/ShardNodeName are compressed.
SICM_READ_WRITE = 2;
// SICM_READ_WRITE_COMPRESSED + conversion from uncompress to compressed
// form is started when TIndexTabletActor tablet starts.
SICM_READ_WRITE_CONVERT = 3;
}
SICM_READ_WRITE_CONVERT is not implemented in this PR.
Issue
#2639