Skip to content
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

asim: enable event series generation #109602

Merged
merged 2 commits into from
Sep 1, 2023
Merged

Conversation

wenyihu6
Copy link
Contributor

@wenyihu6 wenyihu6 commented Aug 28, 2023

asim: enable event series generation

This patch enables interesting event series to be generated and scheduled as
part of the simulation.

The following command is now supported:
"rand_events" [type={cycle_via_hardcoded_survival_goals}]
[duration_to_assert_on_event=<time.Duration>]
e.g. rand_events type=cycle_via_hardcoded_survival_goals duration=5m

  • rand_events: generates interesting event series to be scheduled in the
    simulation.
  • type: type of event series to be scheduled.
  • duration_to_assert_on_event: delay to add the assertion events post
    mutation events for mutation-assertion event series.

asim: better output for executed events

Previously, the randomized testing framework only reports number of scheduled
mutation and assertion events, offering no insights on their execution or the
results of the assertion events. This patch adds more output info to include
executed mutation events, conducted assertion checks, assertion results, and
their execution timing.

New verbosity flags for eval is now supported to print these info.

"eval" [verbose=events]

Part of: #106192
Release Note: none

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@wenyihu6
Copy link
Contributor Author

Please only review the last two commits. The preceding commits are from #109316.

@wenyihu6 wenyihu6 changed the title Sepnewprint asim: enable event series generation Aug 28, 2023
@wenyihu6 wenyihu6 force-pushed the sepnewprint branch 2 times, most recently from eed4fd2 to fe991c8 Compare August 28, 2023 17:44
@wenyihu6 wenyihu6 self-assigned this Aug 28, 2023
@wenyihu6 wenyihu6 force-pushed the sepnewprint branch 2 times, most recently from c1b4669 to 4c56b69 Compare August 28, 2023 19:45
@wenyihu6 wenyihu6 requested a review from kvoli August 28, 2023 22:44
@wenyihu6 wenyihu6 force-pushed the sepnewprint branch 3 times, most recently from c01a590 to c4c7234 Compare August 29, 2023 19:29
Copy link
Collaborator

@kvoli kvoli left a comment

Choose a reason for hiding this comment

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

Looks good, lets discuss how we want to handle the CI timeout offline.

Reviewed 15 of 25 files at r1, 9 of 9 files at r2, 13 of 13 files at r3, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @wenyihu6)


-- commits line 49 at r2:
nit: the wording of this line seems off.


pkg/kv/kvserver/asim/event/mutation_event.go line 64 at r3 (raw file):

func (se SetSpanConfigEvent) String() string {
	return fmt.Sprintf("set span config event with span=%s, config=%s", se.Span.String(), se.Config.String())

Is the .String() call necessary on these? My understanding was that these were inferred by the format function if they exist.


pkg/kv/kvserver/asim/event/mutation_event.go line 99 at r3 (raw file):

func (sne SetNodeLivenessEvent) String() string {
	return fmt.Sprintf("set node liveness event with nodeID=%d, liveness_status=%s", sne.NodeId, sne.LivenessStatus.String())

Same as above.


pkg/kv/kvserver/asim/event/mutation_event.go line 110 at r3 (raw file):

func (sce SetCapacityOverrideEvent) String() string {
	return fmt.Sprintf("set capacity override event with storeID=%d, capacity_override=%s", sce.StoreID, sce.CapacityOverride.String())

Same as above.

This patch enables interesting event series to be generated and scheduled as
part of the simulation.

The following command is now supported:
"rand_events" [type=<string>{cycle_via_hardcoded_survival_goals}]
 [duration_to_assert_on_event=<time.Duration>]
 e.g. rand_events type=cycle_via_hardcoded_survival_goals duration=5m
 - rand_events: generates interesting event series to be scheduled in the
 simulation.
 - type: type of event series to be scheduled.
 - duration_to_assert_on_event: delay to add the assertion events post
   mutation events for mutation-assertion event series.

Part of: cockroachdb#106192
Release note: none
@wenyihu6 wenyihu6 force-pushed the sepnewprint branch 2 times, most recently from 82c97fd to 875e7fe Compare August 30, 2023 20:58
@wenyihu6
Copy link
Contributor Author

pkg/kv/kvserver/asim/event/mutation_event.go line 64 at r3 (raw file):

Previously, kvoli (Austen) wrote…

Is the .String() call necessary on these? My understanding was that these were inferred by the format function if they exist.

opps forgot about it

@wenyihu6
Copy link
Contributor Author

pkg/kv/kvserver/asim/event/mutation_event.go line 99 at r3 (raw file):

Previously, kvoli (Austen) wrote…

Same as above.

Done.

@wenyihu6
Copy link
Contributor Author

pkg/kv/kvserver/asim/event/mutation_event.go line 110 at r3 (raw file):

Previously, kvoli (Austen) wrote…

Same as above.

Done.

@wenyihu6 wenyihu6 marked this pull request as ready for review August 30, 2023 21:00
@wenyihu6 wenyihu6 requested a review from a team as a code owner August 30, 2023 21:00
@wenyihu6 wenyihu6 requested a review from kvoli August 30, 2023 21:00
Copy link
Collaborator

@kvoli kvoli left a comment

Choose a reason for hiding this comment

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

:lgtm: Discussed offline, we will resolve the timing out tests before merging.

Reviewed 29 of 29 files at r4, 12 of 28 files at r5, 1 of 1 files at r6, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @wenyihu6)

Previously, the randomized testing framework only reports number of scheduled
mutation and assertion events, offering no insights on their execution or the
results of the assertion events. This patch adds more output info to include
executed mutation events, conducted assertion checks, assertion results, and
their execution timing.

New verbosity flags for eval is now supported to print these info.
"eval" [verbose=events]

Part of: cockroachdb#106192
Release note: none
@wenyihu6
Copy link
Contributor Author

pkg/kv/kvserver/asim/tests/testdata/rand/events line 4 at r7 (raw file):

----

change_static_option ranges=1

I forgot to shorten the duration when changing the number of ranges generated to 1. After making this change, the time it took to stabilize decreased significantly. The test only takes 2.5 seconds now. I have another PR to remove some of the log events that were time-consuming. But this is no longer blocking this PR. Happy to close this other PR if we prefer to keep the log events for tracing.

@wenyihu6
Copy link
Contributor Author

pkg/kv/kvserver/asim/tests/testdata/rand/events line 4 at r7 (raw file):

Previously, wenyihu6 (Wenyi Hu) wrote…

I forgot to shorten the duration when changing the number of ranges generated to 1. After making this change, the time it took to stabilize decreased significantly. The test only takes 2.5 seconds now. I have another PR to remove some of the log events that were time-consuming. But this is no longer blocking this PR. Happy to close this other PR if we prefer to keep the log events for tracing.

link to this other PR - #109804

@wenyihu6
Copy link
Contributor Author

wenyihu6 commented Sep 1, 2023

TFTR!

bors r=kvoli

@craig
Copy link
Contributor

craig bot commented Sep 1, 2023

Build succeeded:

@craig craig bot merged commit ab57260 into cockroachdb:master Sep 1, 2023
@wenyihu6 wenyihu6 deleted the sepnewprint branch October 30, 2023 17:33
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