Skip to content

Remove KIKIMR_PDISK_ENABLE_T1HA_HASH_WRITING #6439

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

Merged
merged 2 commits into from
Jul 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions ydb/core/base/compile_time_flags.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,3 @@
#ifndef KIKIMR_ALLOW_SSREPLICA_PROBES
#define KIKIMR_ALLOW_SSREPLICA_PROBES 0
#endif
// This feature flag enables PDisk to use t1ha hash in sector footer checksums
#ifndef KIKIMR_PDISK_ENABLE_T1HA_HASH_WRITING
#define KIKIMR_PDISK_ENABLE_T1HA_HASH_WRITING true
#endif
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ void TCompletionLogWrite::Release(TActorSystem *actorSystem) {

TCompletionChunkReadPart::TCompletionChunkReadPart(TPDisk *pDisk, TIntrusivePtr<TChunkRead> &read, ui64 rawReadSize,
ui64 payloadReadSize, ui64 commonBufferOffset, TCompletionChunkRead *cumulativeCompletion, bool isTheLastPart,
const TControlWrapper& useT1ha0Hasher, NWilson::TSpan&& span)
NWilson::TSpan&& span)
: TCompletionAction()
, PDisk(pDisk)
, Read(read)
Expand All @@ -126,7 +126,6 @@ TCompletionChunkReadPart::TCompletionChunkReadPart(TPDisk *pDisk, TIntrusivePtr<
, CumulativeCompletion(cumulativeCompletion)
, Buffer(PDisk->BufferPool->Pop())
, IsTheLastPart(isTheLastPart)
, UseT1ha0Hasher(useT1ha0Hasher)
, Span(std::move(span))
{
if (!IsTheLastPart) {
Expand Down Expand Up @@ -192,7 +191,7 @@ void TCompletionChunkReadPart::Exec(TActorSystem *actorSystem) {
format, actorSystem, PDisk->PDiskActor, PDisk->PDiskId, &PDisk->Mon, PDisk->BufferPool.Get());
ui64 lastNonce = Min((ui64)0, chunkNonce - 1);
restorator.Restore(source, format.Offset(Read->ChunkIdx, sectorIdx), format.MagicDataChunk, lastNonce,
UseT1ha0Hasher, Read->Owner);
Read->Owner);

const ui32 sectorCount = 1;
if (restorator.GoodSectorCount != sectorCount) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,12 +224,11 @@ class TCompletionChunkReadPart : public TCompletionAction {
TCompletionChunkRead *CumulativeCompletion;
TBuffer::TPtr Buffer;
bool IsTheLastPart;
TControlWrapper UseT1ha0Hasher;
NWilson::TSpan Span;
public:
TCompletionChunkReadPart(TPDisk *pDisk, TIntrusivePtr<TChunkRead> &read, ui64 rawReadSize, ui64 payloadReadSize,
ui64 commonBufferOffset, TCompletionChunkRead *cumulativeCompletion, bool isTheLastPart,
const TControlWrapper& useT1ha0Hasher, NWilson::TSpan&& span);
ui64 commonBufferOffset, TCompletionChunkRead *cumulativeCompletion, bool isTheLastPart,
NWilson::TSpan&& span);


bool CanHandleResult() const override {
Expand Down
2 changes: 0 additions & 2 deletions ydb/core/blobstorage/pdisk/blobstorage_pdisk_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,6 @@ struct TPDiskConfig : public TThrRefBase {
ui32 BufferPoolBufferCount = 256;
ui32 MaxQueuedCompletionActions = 128; // BufferPoolBufferCount / 2;
bool UseSpdkNvmeDriver;
TControlWrapper UseT1ha0HashInFooter;

ui64 ExpectedSlotCount = 0;

Expand Down Expand Up @@ -161,7 +160,6 @@ struct TPDiskConfig : public TThrRefBase {
, PDiskGuid(pDiskGuid)
, PDiskId(pdiskId)
, PDiskCategory(pDiskCategory)
, UseT1ha0HashInFooter(KIKIMR_PDISK_ENABLE_T1HA_HASH_WRITING, 0, 1)
{
Initialize();
}
Expand Down
25 changes: 3 additions & 22 deletions ydb/core/blobstorage/pdisk/blobstorage_pdisk_crypto.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,7 @@ namespace NPDisk {
////////////////////////////////////////////////////////////////////////////

class TPDiskHashCalculator : public THashCalculator {
bool UseT1ha0Hasher;

public:
TPDiskHashCalculator(bool useT1ha0Hasher)
: UseT1ha0Hasher(useT1ha0Hasher)
{}

void SetUseT1ha0Hasher(bool x) {
UseT1ha0Hasher = x;
};

ui64 OldHashSector(const ui64 sectorOffset, const ui64 magic, const ui8 *sector,
const ui32 sectorSize) {
REQUEST_VALGRIND_CHECK_MEM_IS_DEFINED(&sectorOffset, sizeof sectorOffset);
Expand Down Expand Up @@ -50,25 +40,16 @@ class TPDiskHashCalculator : public THashCalculator {

ui64 HashSector(const ui64 sectorOffset, const ui64 magic, const ui8 *sector,
const ui32 sectorSize) {
if (UseT1ha0Hasher) {
return T1ha0HashSector<TT1ha0NoAvxHasher>(sectorOffset, magic, sector, sectorSize);
} else {
return OldHashSector(sectorOffset, magic, sector, sectorSize);
}
return T1ha0HashSector<TT1ha0NoAvxHasher>(sectorOffset, magic, sector, sectorSize);
}

bool CheckSectorHash(const ui64 sectorOffset, const ui64 magic, const ui8 *sector,
const ui32 sectorSize, const ui64 sectorHash) {
// On production servers may be two versions.
// If by default used OldHash version, then use it first
// If by default used T1ha0NoAvx version, then use it
if (UseT1ha0Hasher) {
return sectorHash == T1ha0HashSector<TT1ha0NoAvxHasher>(sectorOffset, magic, sector, sectorSize)
|| sectorHash == OldHashSector(sectorOffset, magic, sector, sectorSize);
} else {
return sectorHash == OldHashSector(sectorOffset, magic, sector, sectorSize)
|| sectorHash == T1ha0HashSector<TT1ha0NoAvxHasher>(sectorOffset, magic, sector, sectorSize);
}
return sectorHash == T1ha0HashSector<TT1ha0NoAvxHasher>(sectorOffset, magic, sector, sectorSize)
|| sectorHash == OldHashSector(sectorOffset, magic, sector, sectorSize);
}
};

Expand Down
8 changes: 4 additions & 4 deletions ydb/core/blobstorage/pdisk/blobstorage_pdisk_data.h
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,7 @@ struct TDiskFormat {

void InitMagic() {
MagicFormatChunk = MagicFormatChunkId;
NPDisk::TPDiskHashCalculator hash(false);
NPDisk::TPDiskHashCalculator hash;
hash.Hash(&Guid, sizeof(Guid));
hash.Hash(&MagicNextLogChunkReferenceId, sizeof(MagicNextLogChunkReferenceId));
MagicNextLogChunkReference = hash.GetHashResult();
Expand All @@ -686,7 +686,7 @@ struct TDiskFormat {
}

bool IsHashOk(ui64 bufferSize) const {
NPDisk::TPDiskHashCalculator hashCalculator(false);
NPDisk::TPDiskHashCalculator hashCalculator;
if (Version == 2) {
ui64 size = (char*)&HashVersion2 - (char*)this;
hashCalculator.Hash(this, size);
Expand All @@ -709,7 +709,7 @@ struct TDiskFormat {
void SetHash() {
// Set an invalid HashVersion2 to prevent Version2 code from trying to read incompatible disks
{
NPDisk::TPDiskHashCalculator hashCalculator(false);
NPDisk::TPDiskHashCalculator hashCalculator;
ui64 size = (char*)&HashVersion2 - (char*)this;
hashCalculator.Hash(this, size);
HashVersion2 = hashCalculator.GetHashResult();
Expand All @@ -718,7 +718,7 @@ struct TDiskFormat {
}
// Set Hash
{
NPDisk::TPDiskHashCalculator hashCalculator(false);
NPDisk::TPDiskHashCalculator hashCalculator;
Y_ABORT_UNLESS(DiskFormatSize > sizeof(THash));
ui64 size = DiskFormatSize - sizeof(THash);
hashCalculator.Hash(this, size);
Expand Down
9 changes: 4 additions & 5 deletions ydb/core/blobstorage/pdisk/blobstorage_pdisk_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -838,7 +838,7 @@ bool TPDisk::ChunkWritePiece(TChunkWrite *evChunkWrite, ui32 pieceShift, ui32 pi
ui32 dataChunkSizeSectors = Format.ChunkSize / Format.SectorSize;
TChunkWriter writer(Mon, *BlockDevice.Get(), Format, state.CurrentNonce, Format.ChunkKey, BufferPool.Get(),
desiredSectorIdx, dataChunkSizeSectors, Format.MagicDataChunk, chunkIdx, nullptr, desiredSectorIdx,
nullptr, ActorSystem, PDiskId, &DriveModel, Cfg->UseT1ha0HashInFooter, Cfg->EnableSectorEncryption);
nullptr, ActorSystem, PDiskId, &DriveModel, Cfg->EnableSectorEncryption);

guard.Release();

Expand Down Expand Up @@ -1012,7 +1012,7 @@ TPDisk::EChunkReadPieceResult TPDisk::ChunkReadPiece(TIntrusivePtr<TChunkRead> &
NWilson::TSpan span(TWilson::PDiskBasic, std::move(traceId), "PDisk.CompletionChunkReadPart", NWilson::EFlags::NONE, ActorSystem);
traceId = span.GetTraceId();
THolder<TCompletionChunkReadPart> completion(new TCompletionChunkReadPart(this, read, bytesToRead,
payloadBytesToRead, payloadOffset, read->FinalCompletion, isTheLastPart, Cfg->UseT1ha0HashInFooter, std::move(span)));
payloadBytesToRead, payloadOffset, read->FinalCompletion, isTheLastPart, std::move(span)));
completion->CostNs = DriveModel.TimeForSizeNs(bytesToRead, read->ChunkIdx, TDriveModel::OP_TYPE_READ);
Y_ABORT_UNLESS(bytesToRead <= completion->GetBuffer()->Size());
ui8 *data = completion->GetBuffer()->Data();
Expand Down Expand Up @@ -1620,7 +1620,7 @@ void TPDisk::WriteApplyFormatRecord(TDiskFormat format, const TKey &mainKey) {
bool encrypt = true; // Always write encrypter format because some tests use wrong main key to initiate errors
TSysLogWriter formatWriter(Mon, *BlockDevice.Get(), Format, nonce, mainKey, BufferPool.Get(),
0, ReplicationFactor, Format.MagicFormatChunk, 0, nullptr, 0, nullptr, ActorSystem, PDiskId,
&DriveModel, Cfg->UseT1ha0HashInFooter, encrypt);
&DriveModel, encrypt);

if (format.IsFormatInProgress()) {
// Fill first bytes with magic pattern
Expand Down Expand Up @@ -1708,7 +1708,7 @@ void TPDisk::WriteDiskFormat(ui64 diskSizeBytes, ui32 sectorSizeBytes, ui32 user
// Fill the cyclic log with initial SysLogRecords
SysLogger.Reset(new TSysLogWriter(Mon, *BlockDevice.Get(), Format, SysLogRecord.Nonces.Value[NonceSysLog],
Format.SysLogKey, BufferPool.Get(), firstSectorIdx, endSectorIdx, Format.MagicSysLogChunk, 0,
nullptr, firstSectorIdx, nullptr, ActorSystem, PDiskId, &DriveModel, Cfg->UseT1ha0HashInFooter,
nullptr, firstSectorIdx, nullptr, ActorSystem, PDiskId, &DriveModel,
Cfg->EnableSectorEncryption));

bool isFull = false;
Expand Down Expand Up @@ -2590,7 +2590,6 @@ bool TPDisk::Initialize(TActorSystem *actorSystem, const TActorId &pDiskActor) {
REGISTER_LOCAL_CONTROL(ForsetiMaxLogBatchNs);
REGISTER_LOCAL_CONTROL(ForsetiOpPieceSizeSsd);
REGISTER_LOCAL_CONTROL(ForsetiOpPieceSizeRot);
REGISTER_LOCAL_CONTROL(Cfg->UseT1ha0HashInFooter);

if (Cfg->SectorMap) {
auto diskModeParams = Cfg->SectorMap->GetDiskModeParams();
Expand Down
1 change: 0 additions & 1 deletion ydb/core/blobstorage/pdisk/blobstorage_pdisk_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,6 @@ class TPDisk : public IPDisk {
ui64 SysLogLsn = 0;
TNonceSet LoggedNonces; // Latest on-disk Nonce set
ui64 CostLimitNs;
TControlWrapper UseT1ha0HashInFooter;

TDriveData DriveData;
TAtomic EstimatedLogChunkIdx = 0; // For cost estimation only TDriveData DriveData;
Expand Down
6 changes: 3 additions & 3 deletions ydb/core/blobstorage/pdisk/blobstorage_pdisk_impl_log.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ void TPDisk::InitSysLogger() {
SysLogger.Reset(new TSysLogWriter(Mon, *BlockDevice.Get(), Format,
SysLogRecord.Nonces.Value[NonceSysLog], Format.SysLogKey, BufferPool.Get(),
beginSectorIdx, endSectorIdx, Format.MagicSysLogChunk, 0, nullptr, writeSectorIdx, nullptr, ActorSystem, PDiskId,
&DriveModel, Cfg->UseT1ha0HashInFooter, Cfg->EnableSectorEncryption));
&DriveModel, Cfg->EnableSectorEncryption));
}

bool TPDisk::InitCommonLogger() {
Expand All @@ -67,7 +67,7 @@ bool TPDisk::InitCommonLogger() {
CommonLogger.Reset(new TLogWriter(Mon, *BlockDevice.Get(), Format,
SysLogRecord.Nonces.Value[NonceLog], Format.LogKey, BufferPool.Get(), 0, UsableSectorsPerLogChunk(),
Format.MagicLogChunk, chunkIdx, info, std::min(sectorIdx, UsableSectorsPerLogChunk()),
InitialTailBuffer, ActorSystem, PDiskId, &DriveModel, Cfg->UseT1ha0HashInFooter, Cfg->EnableSectorEncryption));
InitialTailBuffer, ActorSystem, PDiskId, &DriveModel, Cfg->EnableSectorEncryption));
InitialTailBuffer = nullptr;
if (sectorIdx >= UsableSectorsPerLogChunk()) {
if (!AllocateLogChunks(1, 0, OwnerSystem, 0, EOwnerGroupType::Static, true)) {
Expand Down Expand Up @@ -1290,7 +1290,7 @@ void TPDisk::MarkChunksAsReleased(TReleaseChunks& req) {
ui32 dataChunkSizeSectors = Format.ChunkSize / Format.SectorSize;
TLogWriter writer(Mon, *BlockDevice.Get(), Format, nonce, Format.LogKey, BufferPool.Get(), desiredSectorIdx,
dataChunkSizeSectors, Format.MagicLogChunk, req.GapStart->ChunkIdx, nullptr, desiredSectorIdx,
nullptr, ActorSystem, PDiskId, &DriveModel, Cfg->UseT1ha0HashInFooter, Cfg->EnableSectorEncryption);
nullptr, ActorSystem, PDiskId, &DriveModel, Cfg->EnableSectorEncryption);

Y_VERIFY_S(req.GapEnd->DesiredPrevChunkLastNonce, "PDiskId# " << PDiskId
<< "Zero GapEnd->DesiredPrevChunkLastNonce, chunkInfo# " << *req.GapEnd);
Expand Down
6 changes: 3 additions & 3 deletions ydb/core/blobstorage/pdisk/blobstorage_pdisk_logreader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -799,7 +799,7 @@ bool TLogReader::ProcessSectorSet(TSectorData *sector) {
const ui64 magic = format.MagicLogChunk;
TSectorRestorator restorator(false, LogErasureDataParts, false, format,
PDisk->ActorSystem, PDisk->PDiskActor, PDisk->PDiskId, &PDisk->Mon, PDisk->BufferPool.Get());
restorator.Restore(sector->GetData(), sector->Offset, magic, LastNonce, PDisk->Cfg->UseT1ha0HashInFooter, Owner);
restorator.Restore(sector->GetData(), sector->Offset, magic, LastNonce, Owner);

if (!restorator.GoodSectorFlags) {
if (IsInitial) {
Expand Down Expand Up @@ -1117,7 +1117,7 @@ bool TLogReader::ProcessSectorSet(TSectorData *sector) {

void TLogReader::ReplyOk() {
{
TPDiskHashCalculator hasher(PDisk->Cfg->UseT1ha0HashInFooter);
TPDiskHashCalculator hasher;
TGuard<TMutex> guard(PDisk->StateMutex);
if (!IsInitial) {
TOwnerData &ownerData = PDisk->OwnerData[Owner];
Expand Down Expand Up @@ -1188,7 +1188,7 @@ bool TLogReader::ProcessNextChunkReference(TSectorData& sector) {
PDisk->Format, PDisk->ActorSystem, PDisk->PDiskActor, PDisk->PDiskId, &PDisk->Mon,
PDisk->BufferPool.Get());
restorator.Restore(sector.GetData(), sector.Offset, format.MagicNextLogChunkReference, LastNonce,
PDisk->Cfg->UseT1ha0HashInFooter, Owner);
Owner);
LOG_DEBUG_S(*PDisk->ActorSystem, NKikimrServices::BS_PDISK, SelfInfo() << " ProcessNextChunkReference");

if (restorator.LastGoodIdx < ReplicationFactor) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@ TSectorRestorator::TSectorRestorator(const bool isTrippleCopy, const ui32 erasur
{}

void TSectorRestorator::Restore(ui8 *source, const ui64 offset, const ui64 magic, const ui64 lastNonce,
const bool useT1ha0Hash, TOwner owner) {
TOwner owner) {
ui32 sectorCount = IsErasureEncode ? (IsTrippleCopy ? ReplicationFactor : (ErasureDataParts + 1)) : 1;
ui64 maxNonce = 0;
TPDiskHashCalculator hasher(useT1ha0Hash);
TPDiskHashCalculator hasher;
for (ui32 i = 0; i < sectorCount; ++i) {
TDataSectorFooter *sectorFooter = (TDataSectorFooter*)
(source + (i + 1) * Format.SectorSize - sizeof(TDataSectorFooter));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ struct TSectorRestorator {
const bool isErasureEncode, const TDiskFormat &format);


void Restore(ui8 *source, const ui64 offset, const ui64 magic, const ui64 lastNonce, const bool useT1ha0Hash,
TOwner owner);
void Restore(ui8 *source, const ui64 offset, const ui64 magic, const ui64 lastNonce, TOwner owner);

void WriteSector(ui8 *sectorData, ui64 writeOffset);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ void TSysLogReader::RestoreSectorSets() {
const bool isErasureEncode = format.IsErasureEncodeSysLog();
TSectorRestorator restorator(true, LogErasureDataParts, isErasureEncode, format,
PDisk->ActorSystem, PDisk->PDiskActor, PDisk->PDiskId, &PDisk->Mon, PDisk->BufferPool.Get());
restorator.Restore(sectorSetData, sectorIdx * format.SectorSize, magic, 0, PDisk->Cfg->UseT1ha0HashInFooter, 0);
restorator.Restore(sectorSetData, sectorIdx * format.SectorSize, magic, 0, 0);

if (!restorator.GoodSectorFlags) {
continue;
Expand Down
2 changes: 1 addition & 1 deletion ydb/core/blobstorage/pdisk/blobstorage_pdisk_tools.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ bool ReadPDiskFormatInfo(const TString &path, const NPDisk::TMainKey &mainKey, T
(sector + format.SectorSize - sizeof(NPDisk::TDataSectorFooter));

ui64 sectorOffset = sysLogOffset + (ui64)((idx / 3) * 3) * (ui64)format.SectorSize;
bool isCrcOk = NPDisk::TPDiskHashCalculator(KIKIMR_PDISK_ENABLE_T1HA_HASH_WRITING).CheckSectorHash(
bool isCrcOk = NPDisk::TPDiskHashCalculator().CheckSectorHash(
sectorOffset, format.MagicSysLogChunk, sector, format.SectorSize, logFooter->Hash);
outInfo.SectorInfo.push_back(TPDiskInfo::TSectorInfo(logFooter->Nonce, logFooter->Version, isCrcOk));
}
Expand Down
1 change: 0 additions & 1 deletion ydb/core/blobstorage/pdisk/blobstorage_pdisk_ut_run.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ void Run(TVector<IActor*> tests, TTestRunConfig runCfg) {
pDiskConfig->ChunkSize = runCfg.ChunkSize;
pDiskConfig->SectorMap = runCfg.TestContext->SectorMap;
pDiskConfig->EnableSectorEncryption = !pDiskConfig->SectorMap;
pDiskConfig->UseT1ha0HashInFooter = runCfg.UseT1ha0Hasher;
pDiskConfig->FeatureFlags.SetEnableSmallDiskOptimization(false);

NPDisk::TMainKey mainKey{ .Keys = {NPDisk::YdbDefaultPDiskSequence}, .IsInitialized = true };
Expand Down
1 change: 0 additions & 1 deletion ydb/core/blobstorage/pdisk/blobstorage_pdisk_ut_run.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ struct TTestRunConfig {
bool IsBad = false;
bool IsErasureEncodeUserLog = false;
ui32 BeforeTestSleepMs = 100;
bool UseT1ha0Hasher = KIKIMR_PDISK_ENABLE_T1HA_HASH_WRITING;
};

void Run(TVector<IActor*> tests, TTestRunConfig runCfg);
Expand Down
22 changes: 0 additions & 22 deletions ydb/core/blobstorage/pdisk/blobstorage_pdisk_ut_yard.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -145,28 +145,6 @@ YARD_UNIT_TEST(TestSysLogReordering) {
}
}

YARD_UNIT_TEST(TestLogWriteReaDifferentHashers) {
for (ui32 i = 0; i < 4; ++i) {
TTestContext tc(false, true);
TTestRunConfig cfg(&tc);

cfg.UseT1ha0Hasher = i / 2;
Run<TTestLogWriteRead<6000>>(cfg);
cfg.UseT1ha0Hasher = i % 2;
Run<TTestWholeLogRead>(cfg);
}
}

YARD_UNIT_TEST(TestChunkWriteReadDifferentHashers) {
for (ui32 i = 0; i < 2; ++i) {
TTestContext tc(false, true);
TTestRunConfig cfg(&tc);

cfg.UseT1ha0Hasher = i;
Run<TTestChunkWriteRead<1000000, 1500000>>(cfg);
}
}

YARD_UNIT_TEST(TestLogWriteCutUnequal) {
TTestContext tc(false, true);
FillDeviceWithZeroes(&tc, MIN_CHUNK_SIZE);
Expand Down
Loading
Loading