Skip to content

Forbid creating empty tiering rules in column shards #8154

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.

Sign up for GitHub

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

Jump to bottom
Merged
swalrus1 merged 8 commits into from
Aug 23, 2024
Merged

Forbid creating empty tiering rules in column shards #8154

swalrus1 merged 8 commits into from
Aug 23, 2024

Conversation

@swalrus1
Copy link
Collaborator

@swalrus1 swalrus1 commented Aug 22, 2024

  • refactor TTiering
  • don't crash when tiering rules are empty
  • fail when parsing empty tiering rules

@github-actions
Copy link

github-actions bot commented Aug 22, 2024 •
edited
Loading

⚪ 2024-08-22 11:57:00 UTC Pre-commit check for 625e627 has started.
⚪ 2024-08-22 11:59:43 UTC Check linux-x86_64-release-clang14 is running...
⚫ 2024-08-22 12:01:20 UTC Check cancelled

@github-actions
Copy link

github-actions bot commented Aug 22, 2024 •
edited
Loading

⚪ 2024-08-22 11:57:21 UTC Pre-commit check for 625e627 has started.
⚪ 2024-08-22 12:00:02 UTC Check linux-x86_64-relwithdebinfo is running...
⚫ 2024-08-22 12:01:20 UTC Check cancelled

@github-actions
Copy link

github-actions bot commented Aug 22, 2024 •
edited
Loading

⚪ 2024-08-22 11:58:44 UTC Pre-commit check for 625e627 has started.
⚫ 2024-08-22 12:01:24 UTC Check cancelled

@swalrus1 swalrus1 linked an issue Aug 22, 2024 that may be closed by this pull request
Failed verify on applying empty Tiering Rule to OLAP table #8149
Closed
@github-actions
Copy link

github-actions bot commented Aug 22, 2024 •
edited
Loading

⚪ 2024-08-22 12:02:56 UTC Pre-commit check for 9e7073b has started.
⚪ 2024-08-22 12:05:59 UTC Check linux-x86_64-relwithdebinfo is running...
🟡 2024-08-22 13:22:06 UTC Some tests failed, follow the links below. Going to retry failed tests...

Details

Test history | Ya make output

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
14313 12940 0 12 1343 18

⚪ 2024-08-22 13:23:24 UTC Failed tests rerun (try 2) linux-x86_64-relwithdebinfo is running...
🟢 2024-08-22 13:31:06 UTC Tests successful.

Test history | Ya make output

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
30 (only retried tests) 21 0 0 0 9

🟢 2024-08-22 13:31:13 UTC Build successful.
🟡 2024-08-22 13:31:52 UTC ydbd size 8.2 GiB changed* by +117.3 KiB, which is >= 100.0 KiB vs main: Warning

ydbd size dash main: 009889b merge: 9e7073b diff diff %
ydbd size 8 752 037 440 Bytes 8 752 157 552 Bytes +117.3 KiB +0.001%
ydbd stripped size 476 004 872 Bytes 476 006 024 Bytes +1.1 KiB +0.000%

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

@github-actions
Copy link

github-actions bot commented Aug 22, 2024 •
edited
Loading

⚪ 2024-08-22 12:02:58 UTC Pre-commit check for 9e7073b has started.
⚪ 2024-08-22 12:05:46 UTC Check linux-x86_64-release-clang14 is running...
🟢 2024-08-22 12:14:49 UTC Build successful.

@github-actions
Copy link

github-actions bot commented Aug 22, 2024 •
edited
Loading

⚪ 2024-08-22 12:10:19 UTC Pre-commit check for 9e7073b has started.
⚪ 2024-08-22 12:13:05 UTC Check linux-x86_64-release-asan is running...
🔴 2024-08-22 13:40:20 UTC Some tests failed, follow the links below.

Test history | Ya make output

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
9891 9812 0 38 25 16

🟢 2024-08-22 13:41:12 UTC Build successful.
🟡 2024-08-22 13:41:40 UTC ydbd size 5.5 GiB changed* by +780.5 KiB, which is >= 100.0 KiB vs main: Warning

ydbd size dash main: a334994 merge: 9e7073b diff diff %
ydbd size 5 889 521 328 Bytes 5 890 320 576 Bytes +780.5 KiB +0.014%
ydbd stripped size 1 478 898 032 Bytes 1 479 113 872 Bytes +210.8 KiB +0.015%

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

ivanmorozov333
ivanmorozov333 previously approved these changes Aug 22, 2024
View reviewed changes
@github-actions
Copy link

github-actions bot commented Aug 22, 2024 •
edited
Loading

⚪ 2024-08-22 14:27:07 UTC Pre-commit check for 2521408 has started.
⚪ 2024-08-22 14:30:58 UTC Check linux-x86_64-release-clang14 is running...
⚫ 2024-08-22 14:36:01 UTC Check cancelled

@github-actions
Copy link

github-actions bot commented Aug 22, 2024 •
edited
Loading

⚪ 2024-08-22 14:27:25 UTC Pre-commit check for 2521408 has started.
⚪ 2024-08-22 14:30:08 UTC Check linux-x86_64-release-asan is running...
⚫ 2024-08-22 14:36:12 UTC Check cancelled

@github-actions
Copy link

github-actions bot commented Aug 22, 2024 •
edited
Loading

⚪ 2024-08-22 14:32:08 UTC Pre-commit check for 2521408 has started.
⚫ 2024-08-22 14:35:54 UTC Check cancelled

@github-actions
Copy link

github-actions bot commented Aug 22, 2024 •
edited
Loading

⚪ 2024-08-22 14:37:35 UTC Pre-commit check for 1c6e2bb has started.
⚪ 2024-08-22 14:40:17 UTC Check linux-x86_64-release-asan is running...
🔴 2024-08-22 16:07:18 UTC Some tests failed, follow the links below.

Test history | Ya make output

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
9907 9827 0 35 28 17

🟢 2024-08-22 16:08:12 UTC Build successful.
🟢 2024-08-22 16:08:41 UTC ydbd size 5.5 GiB changed* by +18.6 KiB, which is < 100.0 KiB vs main: OK

ydbd size dash main: c5366d7 merge: 1c6e2bb diff diff %
ydbd size 5 890 837 096 Bytes 5 890 856 128 Bytes +18.6 KiB +0.000%
ydbd stripped size 1 479 175 280 Bytes 1 479 179 184 Bytes +3.8 KiB +0.000%

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

@github-actions
Copy link

github-actions bot commented Aug 22, 2024 •
edited
Loading

⚪ 2024-08-22 14:39:51 UTC Pre-commit check for 1c6e2bb has started.
⚪ 2024-08-22 14:42:34 UTC Check linux-x86_64-release-clang14 is running...
🟢 2024-08-22 14:48:38 UTC Build successful.

@github-actions
Copy link

github-actions bot commented Aug 22, 2024 •
edited
Loading

⚪ 2024-08-22 14:41:25 UTC Pre-commit check for 1c6e2bb has started.
⚪ 2024-08-22 14:44:17 UTC Check linux-x86_64-relwithdebinfo is running...
🟡 2024-08-22 15:54:28 UTC Some tests failed, follow the links below. Going to retry failed tests...

Details

Test history | Ya make output

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
14301 12937 0 8 1342 14

⚪ 2024-08-22 15:55:41 UTC Failed tests rerun (try 2) linux-x86_64-relwithdebinfo is running...
🟢 2024-08-22 16:02:41 UTC Tests successful.

Test history | Ya make output

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
23 (only retried tests) 15 0 0 0 8

🟢 2024-08-22 16:02:49 UTC Build successful.
🟢 2024-08-22 16:03:24 UTC ydbd size 8.2 GiB changed* by +30.3 KiB, which is < 100.0 KiB vs main: OK

ydbd size dash main: c5366d7 merge: 1c6e2bb diff diff %
ydbd size 8 753 289 152 Bytes 8 753 320 176 Bytes +30.3 KiB +0.000%
ydbd stripped size 476 019 848 Bytes 476 020 936 Bytes +1.1 KiB +0.000%

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

ivanmorozov333
ivanmorozov333 reviewed Aug 23, 2024
View reviewed changes
ydb/core/tx/tiering/manager.cpp Outdated
for (auto&& i : PathIdTiering) {
auto* tiering = snapshotPtr->GetTieringById(i.second);
if (tiering) {
auto* tiering_rule = snapshotPtr->GetTieringById(i.second);
Copy link
Collaborator

@ivanmorozov333 ivanmorozov333 Aug 23, 2024

Choose a reason for hiding this comment

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

style guide

ydb/core/tx/columnshard/engines/scheme/tiering/tier_info.h
[[nodiscard]] bool Add(const std::shared_ptr<TTierInfo>& tier) {
AFL_VERIFY(tier);
if (!TTLColumnName) {
TTLColumnName = tier->GetEvictColumnName();
Copy link
Collaborator

@ivanmorozov333 ivanmorozov333 Aug 23, 2024

Choose a reason for hiding this comment

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

добавить проверку на не пустую строку

ydb/core/tx/tiering/manager.cpp Outdated
tier->SetSerializer(NTiers::ConvertCompression(it->second.GetCompression()));
}
NOlap::TTiering tiering = tiering_rule->BuildOlapTiers();
AFL_VERIFY(tiering.HasTiers());
Copy link
Collaborator

@ivanmorozov333 ivanmorozov333 Aug 23, 2024

Choose a reason for hiding this comment

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

а нельзя проверку в BuildOlapTiers перенести?

@github-actions
Copy link

github-actions bot commented Aug 23, 2024 •
edited
Loading

⚪ 2024-08-23 07:26:50 UTC Pre-commit check for 1f737bd has started.
⚪ 2024-08-23 07:29:35 UTC Check linux-x86_64-release-asan is running...
⚫ 2024-08-23 08:32:17 UTC Check cancelled

@github-actions
Copy link

github-actions bot commented Aug 23, 2024 •
edited
Loading

⚪ 2024-08-23 07:27:17 UTC Pre-commit check for 1f737bd has started.
⚪ 2024-08-23 07:30:05 UTC Check linux-x86_64-relwithdebinfo is running...
🟡 2024-08-23 08:30:53 UTC Some tests failed, follow the links below. Going to retry failed tests...

Details

Test history | Ya make output

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
14313 12923 0 27 1349 14

🟢 2024-08-23 08:32:25 UTC ydbd size 8.2 GiB changed* by +50.1 KiB, which is < 100.0 KiB vs main: OK

ydbd size dash main: eb1276a merge: 1f737bd diff diff %
ydbd size 8 753 305 232 Bytes 8 753 356 504 Bytes +50.1 KiB +0.001%
ydbd stripped size 476 022 792 Bytes 476 024 264 Bytes +1.4 KiB +0.000%

*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation
⚫ 2024-08-23 08:32:27 UTC Check cancelled

@github-actions
Copy link

github-actions bot commented Aug 23, 2024 •
edited
Loading

⚪ 2024-08-23 07:32:49 UTC Pre-commit check for 1f737bd has started.
⚪ 2024-08-23 07:35:33 UTC Check linux-x86_64-release-clang14 is running...
🟢 2024-08-23 07:43:49 UTC Build successful.

@github-actions
Copy link

github-actions bot commented Aug 23, 2024 •
edited
Loading

⚪ 2024-08-23 08:33:57 UTC Pre-commit check for a286514 has started.
⚪ 2024-08-23 08:36:42 UTC Check linux-x86_64-release-clang14 is running...
🟢 2024-08-23 08:42:06 UTC Build successful.

@github-actions
Copy link

github-actions bot commented Aug 23, 2024 •
edited
Loading

⚪ 2024-08-23 11:25:50 UTC Pre-commit check for a286514 has started.
⚪ 2024-08-23 11:28:26 UTC Check linux-x86_64-release-asan is running...
🔴 2024-08-23 12:52:54 UTC Some tests failed, follow the links below.

Test history | Ya make output

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
9906 9833 0 25 28 20

🟢 2024-08-23 12:53:46 UTC Build successful.
🟢 2024-08-23 12:54:16 UTC ydbd size 5.5 GiB changed* by +51.3 KiB, which is < 100.0 KiB vs main: OK

ydbd size dash main: 62a41e4 merge: a286514 diff diff %
ydbd size 5 890 857 720 Bytes 5 890 910 288 Bytes +51.3 KiB +0.001%
ydbd stripped size 1 479 182 192 Bytes 1 479 189 104 Bytes +6.8 KiB +0.000%

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

@github-actions
Copy link

github-actions bot commented Aug 23, 2024 •
edited
Loading

⚪ 2024-08-23 08:36:05 UTC Pre-commit check for a286514 has started.
⚪ 2024-08-23 08:38:54 UTC Check linux-x86_64-relwithdebinfo is running...
🟡 2024-08-23 09:35:18 UTC Some tests failed, follow the links below. Going to retry failed tests...

Details

Test history | Ya make output

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
14314 12902 0 53 1342 17

⚪ 2024-08-23 09:36:35 UTC Failed tests rerun (try 2) linux-x86_64-relwithdebinfo is running...
🟢 2024-08-23 09:45:11 UTC Tests successful.

Test history | Ya make output

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
70 (only retried tests) 61 0 0 0 9

🟢 2024-08-23 09:45:19 UTC Build successful.
🟢 2024-08-23 09:45:57 UTC ydbd size 8.2 GiB changed* by +58.0 KiB, which is < 100.0 KiB vs main: OK

ydbd size dash main: 62a41e4 merge: a286514 diff diff %
ydbd size 8 753 305 120 Bytes 8 753 364 536 Bytes +58.0 KiB +0.001%
ydbd stripped size 476 022 728 Bytes 476 024 520 Bytes +1.8 KiB +0.000%

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

@swalrus1 swalrus1 enabled auto-merge (squash) August 23, 2024 10:41
@swalrus1 swalrus1 merged commit c249b27 into ydb-platform:main Aug 23, 2024
@swalrus1 swalrus1 deleted the fix-crach-on-empty-tiering-rule branch August 23, 2024 13:25
This was referenced Aug 27, 2024
Merged
Merged
rekby pushed a commit to rekby-forks/ydb that referenced this pull request Aug 28, 2024
@swalrus1 @rekby
Forbid creating empty tiering rules in column shards (ydb-platform#8154)
948d45f
@niksaveliev niksaveliev mentioned this pull request Aug 29, 2024
Merged
This was referenced Aug 30, 2024
Merged
Merged
stanislav-shchetinin pushed a commit to stanislav-shchetinin/ydb that referenced this pull request Aug 30, 2024
@swalrus1 @stanislav-shchetinin
Forbid creating empty tiering rules in column shards (ydb-platform#8154)
b9d1908
@siarheivesialou siarheivesialou mentioned this pull request Sep 4, 2024
Merged
This was referenced Sep 9, 2024
Merged
Merged
zverevgeny pushed a commit to zverevgeny/ydb that referenced this pull request Sep 13, 2024
@swalrus1 @zverevgeny
Forbid creating empty tiering rules in column shards (ydb-platform#8154)
ad8b131
zverevgeny pushed a commit to zverevgeny/ydb that referenced this pull request Sep 14, 2024
@swalrus1 @zverevgeny
Forbid creating empty tiering rules in column shards (ydb-platform#8154)
b95ec14
zverevgeny pushed a commit to zverevgeny/ydb that referenced this pull request Sep 15, 2024
@swalrus1 @zverevgeny
Forbid creating empty tiering rules in column shards (ydb-platform#8154)
4b389f3
@zverevgeny zverevgeny mentioned this pull request Sep 16, 2024
Closed
This was referenced Sep 17, 2024
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ivanmorozov333 ivanmorozov333 ivanmorozov333 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Failed verify on applying empty Tiering Rule to OLAP table

2 participants

@swalrus1 @ivanmorozov333