-
Notifications
You must be signed in to change notification settings - Fork 740
Precharge for external blobs in DataShard Read Iterator Keys request (#14707) #15730
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
Conversation
|
🔴 The changelog entry is less than 20 characters or missing. |
|
⚪ DetailsTest history | Ya make output | Test bloat
⚪ DetailsTest history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ DetailsTest history | Ya make output | Test bloat
⚪ DetailsTest history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
| NTable::TRowState rowState; | ||
| rowState.Init(State.Columns.size()); | ||
| NTable::TSelectStats stats; | ||
| txc.DB.Select(TableInfo.LocalTid, key, State.Columns, rowState, stats, 0, State.ReadVersion, GetReadTxMap(), GetReadTxObserver()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Весь смысл вызова Precharge в том, что он чуть-чуть дешевле - не пытается собрать row state, а только потрогать те страницы, которые могут пригодиться на следующей итерации. Вы же делаете и Precharge и Select, т.е. буквально делаете два раза одно и то же разными способами. Оставьте тогда уж только Select, если вам нужна более точная работа?
| TTransactionContext& txc, | ||
| ui32 queryIndex) | ||
| { | ||
| if (UseNewPrecharge) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Очень плохо называть что-либо "New". Мы будем на это переходить? Если это лучше работает, то почему это не включено сразу без флагов "new"?
| NTable::TRowState rowState; | ||
| rowState.Init(State.Columns.size()); | ||
| NTable::TSelectStats stats; | ||
| txc.DB.Select(TableInfo.LocalTid, key, State.Columns, rowState, stats, 0, State.ReadVersion, GetReadTxMap(), GetReadTxObserver()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Это чтение используется исключительно ради Precharge, так что передавать GetReadTxObserver() не нужно - любые обнаруженные конфликты по транзакциям не должны учитываться.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Обсудили голосом и решили что сейчас замержим, а потом поправим (этот код уже в проде в Nebius, поверх него есть другие PR).
…db-platform#14707) (ydb-platform#15730) Co-authored-by: Semyon Danilov <senya@ydb.tech>
…db-platform#14707) (ydb-platform#15730) Co-authored-by: Semyon Danilov <senya@ydb.tech>
…db-platform#14707) (ydb-platform#15730) Co-authored-by: Semyon Danilov <senya@ydb.tech>
Change description
Missed implementation for blobs precharge
Changelog category