Skip to content

New counter for activations (CurrentActivationTimeUsByActivity) #4938

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 3 commits into from
May 28, 2024

Conversation

kruall
Copy link
Collaborator

@kruall kruall commented May 28, 2024

Changelog entry

...

Changelog category

  • Not for changelog (changelog entry is not required)

Additional information

...

@kruall kruall added the area/actorsystem Actor System related issues label May 28, 2024
@kruall kruall requested a review from alexvru May 28, 2024 13:36
@kruall kruall self-assigned this May 28, 2024
@kruall kruall changed the title New counter for activations New counter for activations (CurrentActivationTimeUsByActivity) May 28, 2024
Copy link

github-actions bot commented May 28, 2024

2024-05-28 13:38:19 UTC Pre-commit check for 3221879 has started.
2024-05-28 13:41:08 UTC Build linux-x86_64-relwithdebinfo is running...
🟢 2024-05-28 14:22:06 UTC Build successful.
2024-05-28 14:22:15 UTC Tests are running...
🔴 2024-05-28 16:11:34 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
72984 60269 0 4 12705 6

Copy link

github-actions bot commented May 28, 2024

2024-05-28 13:39:56 UTC Pre-commit check for 3221879 has started.
2024-05-28 13:42:58 UTC Build linux-x86_64-release-asan is running...
🟢 2024-05-28 14:28:24 UTC Build successful.
2024-05-28 14:28:36 UTC Tests are running...
🔴 2024-05-28 16:34:29 UTC Some tests failed, follow the links below.

Test history

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
13789 13666 0 50 58 15

Copy link

github-actions bot commented May 28, 2024

2024-05-28 13:39:57 UTC Pre-commit check for 3221879 has started.
2024-05-28 13:42:57 UTC Build linux-x86_64-release-clang14 is running...
🟢 2024-05-28 14:24:26 UTC Build successful.

@kruall kruall merged commit 157539d into ydb-platform:main May 28, 2024
11 of 17 checks passed
@kruall kruall deleted the exp/new_counter_for_long_activations branch May 28, 2024 16:57
kruall added a commit that referenced this pull request May 29, 2024
@niksaveliev niksaveliev mentioned this pull request May 29, 2024
@StekPerepolnen StekPerepolnen mentioned this pull request May 31, 2024
@@ -165,6 +175,12 @@ namespace NActors {
AggregateOne(ActorsAliveByActivity, other.ActorsAliveByActivity);
AggregateOne(ScheduledEventsByActivity, other.ScheduledEventsByActivity);
AggregateOne(StuckActorsByActivity, other.StuckActorsByActivity);
if (other.CurrentActivationTime.TimeUs) {
Copy link

@drbasic drbasic Jun 11, 2024

Choose a reason for hiding this comment

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

@kruall  в этом  if  thread sanitizer  находит проблему - чтение данных, модифицируемых на другом потоке.
В следующем   if происходит что-то совсем странное (хотя это проходит для tsan незамеченным) - идет обращение к контейнеру, который изменяется на другом потоке. Мне кажется понятно к чему это приводит.

Copy link

@drbasic drbasic Jun 11, 2024

Choose a reason for hiding this comment

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

Обсудили голосом.

  1. гонка по данным происходит когда специально выделенный поток проходится по всем TExecutorThreadStats и аггрегирует их, причем делает это за два прохода.
  2. с AggregatedCurrentActivationTime проблемы не происходит, потому что их может менять только поток, который собирает статистику

т.е. если в первом случае данные в other меняются в других потоках во момент сбора статистики, то с AggregatedCurrentActivationTime такого не происходит, их просто не трогают на тех потоках.

@niksaveliev niksaveliev mentioned this pull request Jun 17, 2024
@StekPerepolnen StekPerepolnen mentioned this pull request Jun 19, 2024
@CyberROFL CyberROFL mentioned this pull request Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/actorsystem Actor System related issues not-for-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants