Skip to content

Messages are written to the partition in two stages #18118

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Alek5andr-Kotov
Copy link
Collaborator

@Alek5andr-Kotov Alek5andr-Kotov commented May 7, 2025

Changelog entry

The logic of writing messages in the partition has changed. In the first iteration, the message is written as is and a response is sent to the user. In the second iteration, the written messages are combined into large blobs.

Changelog category

  • Not for changelog (changelog entry is not required)

Description for reviewers

...

@Alek5andr-Kotov Alek5andr-Kotov requested review from a team as code owners May 7, 2025 09:33
@Alek5andr-Kotov Alek5andr-Kotov requested a review from nshestakov May 7, 2025 09:33
@Alek5andr-Kotov Alek5andr-Kotov changed the title LOGBROKER-9587 WIP: LOGBROKER-9587 May 7, 2025
Copy link

github-actions bot commented May 7, 2025

🟢 2025-05-07 13:18:07 UTC The validation of the Pull Request description is successful.

Copy link

github-actions bot commented May 7, 2025

2025-05-07 12:42:07 UTC Pre-commit check linux-x86_64-relwithdebinfo for f33eb0d has started.
2025-05-07 12:42:14 UTC Artifacts will be uploaded here
🔴 2025-05-07 12:43:57 UTC Graph compare failed, see the logs.

Copy link

github-actions bot commented May 7, 2025

2025-05-07 12:42:08 UTC Pre-commit check linux-x86_64-release-asan for f33eb0d has started.
2025-05-07 12:42:31 UTC Artifacts will be uploaded here
🔴 2025-05-07 12:44:21 UTC Graph compare failed, see the logs.

@Alek5andr-Kotov Alek5andr-Kotov changed the title WIP: LOGBROKER-9587 Messages are written to the partition in two stages May 7, 2025
Copy link

github-actions bot commented May 7, 2025

2025-05-07 13:15:30 UTC Pre-commit check linux-x86_64-relwithdebinfo for 77ec640 has started.
2025-05-07 13:15:41 UTC Artifacts will be uploaded here
2025-05-07 13:18:38 UTC ya make is running...
🔴 2025-05-07 13:24:59 UTC Build failed, see the logs. Also see fail summary

Copy link

github-actions bot commented May 7, 2025

2025-05-07 13:15:46 UTC Pre-commit check linux-x86_64-release-asan for 77ec640 has started.
2025-05-07 13:15:57 UTC Artifacts will be uploaded here
2025-05-07 13:18:47 UTC ya make is running...
🔴 2025-05-07 13:25:17 UTC Build failed, see the logs. Also see fail summary

@@ -392,12 +395,28 @@ void TPartition::AddMetaKey(TEvKeyValue::TEvRequest* request) {
write->SetStorageChannel(NKikimrClient::TKeyValueRequest::INLINE);
}

//bool TPartition::CleanUpFastWrite(TEvKeyValue::TEvRequest* request, const TActorContext& ctx)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

забыл удалить?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Удалил закоментированный код
f8bc181

@@ -434,9 +453,9 @@ bool TPartition::CleanUpBlobs(TEvKeyValue::TEvRequest *request, const TActorCont
break;
}

auto& firstKey = BlobEncoder.DataKeysBody.front();
auto& firstKey = CompactionBlobEncoder.DataKeysBody.front();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
auto& firstKey = CompactionBlobEncoder.DataKeysBody.front();
const auto& firstKey = CompactionBlobEncoder.DataKeysBody.front();

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Согласен. Поменял

@@ -937,15 +956,41 @@ void TPartition::LogAndCollectError(NKikimrServices::EServiceKikimr service, con
LogAndCollectError(error, ctx);
}

const TPartitionBlobEncoder& TPartition::GetPartitionZone(ui64 offset) const
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const TPartitionBlobEncoder& TPartition::GetPartitionZone(ui64 offset) const
const TPartitionBlobEncoder& TPartition::GetPartitionEncoder(ui64 offset) const

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Согласен. Поменял

@@ -2140,7 +2205,8 @@ void TPartition::RunPersist() {
WriteInfosApplied.clear();
//Done with counters.

DumpKeyValueRequest(PersistRequest->Record);
//// иногда во время отладки хочется посмотреть что записывается на диск
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
//// иногда во время отладки хочется посмотреть что записывается на диск
// for debugging purposes

Y_ABORT_UNLESS(Head.PackedSize == 0,
"Head.PackedSize=%" PRIu32, Head.PackedSize);

// если это первая запись
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

надо поперводить все на англицкий

Copy link
Collaborator Author

@Alek5andr-Kotov Alek5andr-Kotov May 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Перевёл
bb4a452

HeadKeys.clear();

// мы только что записали, в теле должно что-то быть
Y_ABORT_UNLESS(!DataKeysBody.empty());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

А могли выше HeadKeys быть пустыми?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Здесь Head.PackedSize != 0 и в голове что-то есть.

auto keys = std::move(dataKeysBody);
dataKeysBody.clear();

auto& cz = Partition()->CompactionBlobEncoder; // Compaction zone
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

А может дать полные названия? а то сложно читается :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Переменные активно используются в этой функции. Мне было неудобно читать с длинными именами. Поэтому сократил.

@@ -452,6 +589,24 @@ TReadAnswer TReadInfo::FormAnswer(
Y_ABORT_UNLESS(blobs.size() == Blobs.size());
response->Check();
bool needStop = false;
#if 1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

а это зачем?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Удалил

return GetNewFastWriteKeyImpl(headCleared, headSize);
}

//std::pair<TKey, ui32> TPartition::GetNewWriteKeyImpl(bool headCleared, bool needCompaction, ui32 headSize)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

и тут какой-то непонятный коммент :)

@@ -2057,6 +2113,14 @@ void TPartition::ProcessCommitQueue() {
}
}

ui64 TPartition::NextReadCookie()
{
if (Cookie == Max<ui64>()) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Смысл метода не понимаю.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Пропускает зарезервированные значения cookie

Copy link

github-actions bot commented May 7, 2025

2025-05-07 14:51:46 UTC Pre-commit check linux-x86_64-release-asan for 52ef492 has started.
2025-05-07 14:52:22 UTC Artifacts will be uploaded here
2025-05-07 14:56:00 UTC ya make is running...
🟡 2025-05-07 16:34:16 UTC Some tests failed, follow the links below. This fail is not in blocking policy yet Going to retry failed tests...

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
14507 14235 0 198 46 28

2025-05-07 16:35:31 UTC ya make is running... (failed tests rerun, try 2)
🟡 2025-05-07 17:15:48 UTC Some tests failed, follow the links below. This fail is not in blocking policy yet Going to retry failed tests...

Test history | Ya make output | Test bloat | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
2091 (only retried tests) 1922 0 116 27 26

2025-05-07 17:16:12 UTC ya make is running... (failed tests rerun, try 3)
🟡 2025-05-07 17:56:03 UTC Some tests failed, follow the links below. This fail is not in blocking policy yet

Test history | Ya make output | Test bloat | Test bloat | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
1835 (only retried tests) 1671 0 105 34 25

🟢 2025-05-07 17:56:19 UTC Build successful.
🟡 2025-05-07 17:56:55 UTC ydbd size 3.8 GiB changed* by +361.3 KiB, which is >= 100.0 KiB vs main: Warning

ydbd size dash main: 0c515e9 merge: 52ef492 diff diff %
ydbd size 4 126 338 008 Bytes 4 126 707 936 Bytes +361.3 KiB +0.009%
ydbd stripped size 1 432 722 240 Bytes 1 432 797 216 Bytes +73.2 KiB +0.005%

*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation

Copy link

github-actions bot commented May 7, 2025

2025-05-07 14:52:58 UTC Pre-commit check linux-x86_64-relwithdebinfo for 52ef492 has started.
2025-05-07 14:53:09 UTC Artifacts will be uploaded here
2025-05-07 14:55:57 UTC ya make is running...
🟡 2025-05-07 16:35:13 UTC Some tests failed, follow the links below. Going to retry failed tests...

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
29487 26595 0 200 2653 39

2025-05-07 16:37:48 UTC ya make is running... (failed tests rerun, try 2)
🟡 2025-05-07 17:09:04 UTC Some tests failed, follow the links below. Going to retry failed tests...

Test history | Ya make output | Test bloat | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
731 (only retried tests) 479 0 188 33 31

2025-05-07 17:09:16 UTC ya make is running... (failed tests rerun, try 3)
🔴 2025-05-07 17:50:53 UTC Some tests failed, follow the links below.

Test history | Ya make output | Test bloat | Test bloat | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
707 (only retried tests) 457 0 189 29 32

🟢 2025-05-07 17:51:02 UTC Build successful.
🟡 2025-05-07 17:51:26 UTC ydbd size 2.2 GiB changed* by +178.3 KiB, which is >= 100.0 KiB vs main: Warning

ydbd size dash main: b139feb merge: 52ef492 diff diff %
ydbd size 2 346 269 176 Bytes 2 346 451 776 Bytes +178.3 KiB +0.008%
ydbd stripped size 493 370 608 Bytes 493 381 040 Bytes +10.2 KiB +0.002%

*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants