Skip to content

*: add CollectProfile method to the Resumer#110556

Merged
craig[bot] merged 5 commits intocockroachdb:masterfrom
adityamaru:new-resumer-method
Sep 20, 2023
Merged

*: add CollectProfile method to the Resumer#110556
craig[bot] merged 5 commits intocockroachdb:masterfrom
adityamaru:new-resumer-method

Conversation

@adityamaru
Copy link
Copy Markdown
Contributor

@adityamaru adityamaru commented Sep 13, 2023

Please refer to individual commits.

Closes: #109671

@adityamaru adityamaru requested a review from a team as a code owner September 13, 2023 13:54
@adityamaru adityamaru requested a review from a team September 13, 2023 13:54
@adityamaru adityamaru requested review from a team as code owners September 13, 2023 13:54
@adityamaru adityamaru requested review from DrewKimball and jayshrivastava and removed request for a team September 13, 2023 13:54
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

// CollectProfile is called when a job has been requested to collect a job
// profile. This profile may contain any information that provides enhanced
// observability into a job's execution.
CollectProfile(ctx context.Context, execCtx interface{}) error
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we want to have every resumer implement this?
Looks like most of them return nil.
Perhaps we can have

type ProfilerCollector interface {
   CollectProfile(....)
}

And have the jobs implement this interface?
When you look at the resumer, check to see if it implemented this method, and call it. If not, return "nil" as the default implementation already does.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think I prefer the implementation actively, explicitly state that it doesn't want to do anything when profiled, rather than implicitly say so by forgetting to implement the method.

This change adds a new method to the Resumer interface
that will be called whenever we request the execution details
for a particular job. This method is currently stubbed for
all resumers but will be filled out for those jobs that have
opted in to jobs profiling.

Informs: cockroachdb#109671
Release note: None
Previously, tracing aggregator stats sent back by all processors in
the flow would be periodically dumped to the job_info table basis an
arbitrary timer. Now, these stats are only persisted to disk when the
job has been instructed to collect a profile.

Informs: cockroachdb#109671
Release note: None
Previously, tracing aggregator stats sent back by all processors in
the flow would be periodically dumped to the job_info table basis an
arbitrary timer. Now, these stats are only persisted to disk when the
job has been instructed to collect a profile.

Informs: cockroachdb#109671
Release note: None
Previously, tracing aggregator stats sent back by all processors in
the flow would be periodically dumped to the job_info table basis an
arbitrary timer. Now, these stats are only persisted to disk when the
job has been instructed to collect a profile.

Informs: cockroachdb#109671
Release note: None
@adityamaru
Copy link
Copy Markdown
Contributor Author

TFTR!

bors r=dt

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Sep 20, 2023

Build failed (retrying...):

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Sep 20, 2023

Build failed (retrying...):

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Sep 20, 2023

Build succeeded:

@craig craig bot merged commit 8bf5ade into cockroachdb:master Sep 20, 2023
@adityamaru adityamaru deleted the new-resumer-method branch September 20, 2023 20:46
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.

jobsprofiler: add ExecutionDetails method to the Resumer interface

4 participants