[MOD-12899] add asm warning in Profile#7913
Conversation
introduce ProfileWarnings
return all warnings in sendchunk resp3 return all warnings as an array in profil;e
add testProfileMultipleWarnings and testPersistProfileWarning_MaxPrefixExpansions and test_multiple_warnings
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
src/profile.c
Outdated
| } | ||
| } else { | ||
| printProfileType("TEXT"); | ||
| } else if (root && root->current) { |
There was a problem hiding this comment.
Is this change related to this PR?
There was a problem hiding this comment.
I have another PR open for a crash found with this change, u can review that one separately
|
Please add a short description and a MOD reference |
JoanFM
changed the title
| ProfileWarnings_Add(&req->profileCtx.warnings, PROFILE_WARNING_TYPE_BG_SCAN_OOM); | ||
| } | ||
| if (req->stateflags & QEXEC_S_ASM_TRIMMING_DELAY_TIMEOUT) { | ||
| QueryWarningsGlobalStats_UpdateWarning(QUERY_WARNING_CODE_ASM_INACCURATE_RESULTS, 1, !IsInternal(req)); |
There was a problem hiding this comment.
How do we know we didn't already log this event to the metric counter in sendChunk?
There was a problem hiding this comment.
I do not quite get it, this is a different code path. Same as other warnings? These are totally different return paths so why would they be called them together? If the concern is about coord/shard separation, there is the bool coord to differentiate the level.
There was a problem hiding this comment.
not completely different paths - this function is also called from _FT.CURSOR PFOILE , which might be triggered after the previous cursor read already passed sendChunck path.
Unlike query oom which is triggered first thing first, and the code goes directly here.
Also for sendChunk itself, if we have multiple cursor reads, is it correct to log the same event to the metric counter each time?
There was a problem hiding this comment.
to me it is ok to log for every ft cursor read one warning event yes.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
…because of ASM and add asm warnings in profile (#7869)(#7899)(#7913) (#8087) * manual backport of ASM warnings * remove some warning * test: change timeout test to not collide with RLTest timeout * asm warnings profile * add new case * test: add condition for source shard to pass migration * add back time sleep to test
2 participants
Describe the changes in the pull request
Handle the warning from ASM in profile
Note
Highlights ASM-related inaccuracy to users and tooling during Atomic Slot Migration.
PROFILE_WARNING_TYPE_ASM_INACCURATE_RESULTStoprofile.h; include it inProfile_Printwarnings and bitset checksaggregate_exec.c, whenQEXEC_S_ASM_TRIMMING_DELAY_TIMEOUTis set, update global stats, addPROFILE_WARNING_TYPE_ASM_INACCURATE_RESULTS, and replyQUERY_ASM_INACCURATE_RESULTSacross RESP2, RESP3, and empty-results paths (including cursor flows and_replyWarnings)tests/pytests/test_asm.pyto verifyFT.CURSOR PROFILEreturns the ASM warning during migration (both single-threaded and BG variants)Written by Cursor Bugbot for commit 9c702cf. This will update automatically on new commits. Configure here.