Skip to content

Fix vector index billing #11174

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 6 commits into from
Nov 5, 2024
Merged

Conversation

MBkkt
Copy link
Contributor

@MBkkt MBkkt commented Nov 1, 2024

For kmeans tree vector index count of reads almost always not equal to count of uploads.

For common secondary index, they're always equal except fail cases.

@MBkkt MBkkt requested review from ijon and CyberROFL November 1, 2024 06:00
@MBkkt MBkkt force-pushed the mbkkt/vector-index-billing branch from f55fae8 to c047a11 Compare November 1, 2024 06:03

This comment was marked as outdated.

This comment was marked as outdated.

@MBkkt MBkkt self-assigned this Nov 1, 2024

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

Comment on lines 142 to 155
TString id = TStringBuilder()
<< buildId << "-"
<< buildInfo.TablePathId.OwnerId << "-" << buildInfo.TablePathId.LocalPathId << "-"
<< billed.GetUploadRows() + billed.GetReadRows() << "-" << billed.GetUploadBytes() + billed.GetReadBytes() << "-"
<< processed.GetUploadRows() + processed.GetReadRows() << "-" << processed.GetUploadBytes() + processed.GetReadBytes();

NIceDb::TNiceDb db(txc.DB);

buildInfo.Billed += toBill;
Self->PersistBuildIndexBilling(db, buildInfo);
billed += toBill;
Self->PersistBuildIndexBilled(db, buildInfo);

ui64 requestUnits = RequestUnits(toBill);

TString id = TStringBuilder()
<< buildId << "-"
<< buildInfo.TablePathId.OwnerId << "-" << buildInfo.TablePathId.LocalPathId << "-"
<< buildInfo.Billed.GetRows() << "-" << buildInfo.Billed.GetBytes() << "-"
<< buildInfo.Processed.GetRows() << "-" << buildInfo.Processed.GetBytes();

const TString billRecord = TBillRecord()
Copy link
Contributor Author

@MBkkt MBkkt Nov 1, 2024

Choose a reason for hiding this comment

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

TString id was generated with kind of typo: buildInfo.Billed was always equal to buildInfo.Processed because we already executed buildInfo.Billed += toBill;

Comment on lines -147 to -151
TString id = TStringBuilder()
<< buildId << "-"
<< buildInfo.TablePathId.OwnerId << "-" << buildInfo.TablePathId.LocalPathId << "-"
<< buildInfo.Billed.GetRows() << "-" << buildInfo.Billed.GetBytes() << "-"
<< buildInfo.Processed.GetRows() << "-" << buildInfo.Processed.GetBytes();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is format of this string fixed? Or it can be any unique string?
If any I suggest using <upload rows>-<upload bytes>-<read rows>-<read bytes> instead of <upload rows + read rows>-<upload bytes + read bytes>

Copy link
Member

Choose a reason for hiding this comment

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

any unique string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I changed it to more detailed format

@@ -786,7 +786,6 @@ struct TSchemeShard::TTxMonitoring : public NTabletFlatExecutor::TTransactionBas
TABLEH() {str << "DebugMessage";}
TABLEH() {str << "SeqNo";}
TABLEH() {str << "Processed";}
TABLEH() {str << "Billed";}
Copy link
Member

Choose a reason for hiding this comment

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

maybe print the number of rows and bytes read/written here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Billed was removed from per shard status some time ago.
I just noticed that it wasn't removed from this line

@MBkkt MBkkt requested a review from CyberROFL November 1, 2024 10:00
Copy link

github-actions bot commented Nov 1, 2024

2024-11-01 10:01:52 UTC Pre-commit check linux-x86_64-release-asan for d2b72cd has started.
2024-11-01 10:02:03 UTC Artifacts will be uploaded here
2024-11-01 10:05:25 UTC ya make is running...
🟡 2024-11-01 11:45:42 UTC Some tests failed, follow the links below. This fail is not in blocking policy yet

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
10676 10598 0 30 15 33

🟢 2024-11-01 11:46:38 UTC Build successful.
🟡 2024-11-01 11:47:08 UTC ydbd size 5.7 GiB changed* by +316.8 KiB, which is >= 100.0 KiB vs main: Warning

ydbd size dash main: ba3eaea merge: d2b72cd diff diff %
ydbd size 6 149 418 152 Bytes 6 149 742 536 Bytes +316.8 KiB +0.005%
ydbd stripped size 1 534 926 864 Bytes 1 534 976 656 Bytes +48.6 KiB +0.003%

*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 Nov 1, 2024

2024-11-01 10:01:57 UTC Pre-commit check linux-x86_64-relwithdebinfo for d2b72cd has started.
2024-11-01 10:02:08 UTC Artifacts will be uploaded here
2024-11-01 10:05:29 UTC ya make is running...
🟡 2024-11-01 11:37: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?
41722 36061 0 9 5554 98

2024-11-01 11:40:41 UTC ya make is running... (failed tests rerun, try 2)
🟡 2024-11-01 11:52:19 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?
109 (only retried tests) 17 0 1 0 91

2024-11-01 11:52:27 UTC ya make is running... (failed tests rerun, try 3)
🔴 2024-11-01 12:03: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?
92 (only retried tests) 0 0 1 0 91

🟢 2024-11-01 12:03:59 UTC Build successful.
🟡 2024-11-01 12:04:22 UTC ydbd size 2.8 GiB changed* by +224.4 KiB, which is >= 100.0 KiB vs main: Warning

ydbd size dash main: ba3eaea merge: d2b72cd diff diff %
ydbd size 3 039 031 368 Bytes 3 039 261 176 Bytes +224.4 KiB +0.008%
ydbd stripped size 481 434 840 Bytes 481 451 928 Bytes +16.7 KiB +0.004%

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

CyberROFL
CyberROFL previously approved these changes Nov 1, 2024
Copy link
Member

@CyberROFL CyberROFL left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link

github-actions bot commented Nov 1, 2024

2024-11-01 13:25:46 UTC Pre-commit check linux-x86_64-release-asan for 3727ec0 has started.
2024-11-01 13:25:56 UTC Artifacts will be uploaded here
2024-11-01 13:29:19 UTC ya make is running...
🟡 2024-11-01 15:18:23 UTC Some tests failed, follow the links below. This fail is not in blocking policy yet

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
10695 10564 0 60 31 40

🟢 2024-11-01 15:19:18 UTC Build successful.
🟡 2024-11-01 15:19:51 UTC ydbd size 5.7 GiB changed* by +320.8 KiB, which is >= 100.0 KiB vs main: Warning

ydbd size dash main: 509064d merge: 3727ec0 diff diff %
ydbd size 6 149 523 352 Bytes 6 149 851 840 Bytes +320.8 KiB +0.005%
ydbd stripped size 1 534 949 648 Bytes 1 535 003 536 Bytes +52.6 KiB +0.004%

*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 Nov 1, 2024

2024-11-01 13:26:35 UTC Pre-commit check linux-x86_64-relwithdebinfo for 3727ec0 has started.
2024-11-01 13:26:47 UTC Artifacts will be uploaded here
2024-11-01 13:30:19 UTC ya make is running...
🟡 2024-11-01 14:59:08 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?
41730 36072 0 5 5550 103

2024-11-01 15:02:46 UTC ya make is running... (failed tests rerun, try 2)
🟡 2024-11-01 15:14:28 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?
102 (only retried tests) 8 0 1 0 93

2024-11-01 15:14:36 UTC ya make is running... (failed tests rerun, try 3)
🔴 2024-11-01 15:34:01 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?
94 (only retried tests) 1 0 1 0 92

🟢 2024-11-01 15:34:08 UTC Build successful.
🟡 2024-11-01 15:34:30 UTC ydbd size 2.8 GiB changed* by +231.9 KiB, which is >= 100.0 KiB vs main: Warning

ydbd size dash main: b3d3a0f merge: 3727ec0 diff diff %
ydbd size 3 039 063 496 Bytes 3 039 301 008 Bytes +231.9 KiB +0.008%
ydbd stripped size 481 437 720 Bytes 481 459 416 Bytes +21.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

@MBkkt MBkkt added the rebase-and-check Rebase PR with the current base branch and check label Nov 5, 2024
@github-actions github-actions bot removed the rebase-and-check Rebase PR with the current base branch and check label Nov 5, 2024
Copy link

github-actions bot commented Nov 5, 2024

2024-11-05 08:44:45 UTC Pre-commit check linux-x86_64-release-asan for 2a22f37 has started.
2024-11-05 08:44:56 UTC Artifacts will be uploaded here
2024-11-05 08:48:28 UTC ya make is running...
🟡 2024-11-05 10:33:35 UTC Some tests failed, follow the links below. This fail is not in blocking policy yet

Test history | Ya make output | Test bloat

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
10677 10609 0 25 10 33

🟢 2024-11-05 10:34:32 UTC Build successful.
🟡 2024-11-05 10:35:02 UTC ydbd size 5.7 GiB changed* by +328.8 KiB, which is >= 100.0 KiB vs main: Warning

ydbd size dash main: 129f0f1 merge: 2a22f37 diff diff %
ydbd size 6 150 316 272 Bytes 6 150 652 936 Bytes +328.8 KiB +0.005%
ydbd stripped size 1 535 122 896 Bytes 1 535 184 976 Bytes +60.6 KiB +0.004%

*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 Nov 5, 2024

2024-11-05 08:44:49 UTC Pre-commit check linux-x86_64-relwithdebinfo for 2a22f37 has started.
2024-11-05 08:45:01 UTC Artifacts will be uploaded here
2024-11-05 08:48:36 UTC ya make is running...
🟡 2024-11-05 10:17:09 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?
41733 36068 0 5 5559 101

2024-11-05 10:20:47 UTC ya make is running... (failed tests rerun, try 2)
🟢 2024-11-05 10:32:44 UTC Tests successful.

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

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
108 (only retried tests) 11 0 0 1 96

🟢 2024-11-05 10:32:51 UTC Build successful.
🟡 2024-11-05 10:33:14 UTC ydbd size 2.8 GiB changed* by +220.4 KiB, which is >= 100.0 KiB vs main: Warning

ydbd size dash main: 129f0f1 merge: 2a22f37 diff diff %
ydbd size 3 039 354 984 Bytes 3 039 580 696 Bytes +220.4 KiB +0.007%
ydbd stripped size 481 618 360 Bytes 481 631 352 Bytes +12.7 KiB +0.003%

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

@MBkkt MBkkt merged commit 87ae150 into ydb-platform:main Nov 5, 2024
13 of 14 checks passed
@MBkkt MBkkt mentioned this pull request Jan 16, 2025
40 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants