Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 37 additions & 31 deletions src/generated/cloud/bigquery/v2/src/operation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,14 +104,14 @@ pub enum JobPollerError {
#[derive(Debug)]
pub struct JobPoller {
policy: JobRetryPolicy,
builder: InsertJob,
builder: Box<InsertJob>,
}

impl JobPoller {
pub(crate) fn new(builder: InsertJob) -> Self {
Self {
policy: JobRetryPolicy::default(),
builder,
builder: Box::new(builder),
}
}

Expand All @@ -128,38 +128,44 @@ impl JobPoller {
}

/// Polls the job until it is done, returning the final Job status.
pub async fn until_done(self) -> Result<Job, JobPollerError> {
let mut attempts = 0_u32;
let mut builder = self.builder;
let backoff = self.policy.backoff;
let start_time = std::time::Instant::now();

loop {
// NOTE: the client library intercepts errors and retries internally
// according to the policies set on `builder`.
let job_result = builder.clone().poller().until_done().await?;
attempts += 1;

if let Some(status) = &job_result.status
&& let Some(err) = &status.error_result
{
if is_retryable_job_error(&err.reason)
&& attempts < self.policy.job_level_attempt_limit
pub fn until_done(
self,
) -> std::pin::Pin<Box<dyn std::future::Future<Output = Result<Job, JobPollerError>> + Send>>

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.

why does the signature has to change that much ? isn't this gonna diverge from other LROs that are async and return just a Result ?

Maybe I don't understand Pin/Box enough to understand what is going on here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was struggling to get this test pass due to stack overflow
https://github.com/googleapis/google-cloud-rust/pull/6232/changes#diff-824e62dc84d6a981c4c62f1f254e61712feacc64d410a3a1b0744f981cb657edR267
https://pantheon.corp.google.com/cloud-build/builds;region=us-central1/6916363e-91b1-4147-81c4-2bf87a03abd7?project=rust-sdk-testing&e=SqlStudioMysqlLaunch::SqlStudioMysqlEnabled

The builder and polling loop state machine get pretty huge on the stack, so boxing it offloads that to the heap.
The the .until_done().await caller syntax is still the same, but it seems like I haven't been able to fix the issue.

{
Box::pin(async move {
let mut attempts = 0_u32;
let mut builder = self.builder;
let backoff = self.policy.backoff;
let start_time = std::time::Instant::now();

loop {
// NOTE: the client library intercepts errors and retries internally
// according to the policies set on `builder`.
let poller = Box::new(builder.clone().poller());
let job_result = poller.until_done().await?;
attempts += 1;

if let Some(status) = &job_result.status
&& let Some(err) = &status.error_result
{
let retry_job = prepare_job_for_retry(job_result);
builder = builder.set_job(retry_job);

let retry_state = RetryState::new(true)
.set_start(start_time)
.set_attempt_count(attempts);
let delay = backoff.on_failure(&retry_state);
tokio::time::sleep(delay).await;
continue;
if is_retryable_job_error(&err.reason)
&& attempts < self.policy.job_level_attempt_limit
{
let retry_job = prepare_job_for_retry(job_result);
*builder = builder.set_job(retry_job);

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.

critical

This assignment will likely cause a compilation error because calling builder.set_job(...) moves the InsertJob out of the Box (moving builder), making it impossible to assign back to *builder. Instead, you should reassign a new Box to builder.

Suggested change
*builder = builder.set_job(retry_job);
builder = Box::new(builder.set_job(retry_job));


let retry_state = RetryState::new(true)
.set_start(start_time)
.set_attempt_count(attempts);
let delay = backoff.on_failure(&retry_state);
tokio::time::sleep(delay).await;
continue;
}
return Err(JobPollerError::ErrorProto(err.clone()));
}
return Err(JobPollerError::ErrorProto(err.clone()));
return Ok(job_result);
}
return Ok(job_result);
}
})
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/lro/src/internal/discovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ where
None
}
async fn until_done(self) -> Result<O> {
crate::until_done(self).await
Box::pin(crate::until_done(self)).await
}

#[cfg(feature = "unstable-stream")]
Expand Down
Loading