mansikhara commented on code in PR #346:
URL: https://github.com/apache/cassandra-sidecar/pull/346#discussion_r3261133348
##########
server/src/main/java/org/apache/cassandra/sidecar/handlers/restore/UpdateRestoreJobHandler.java:
##########
@@ -87,36 +91,40 @@ protected void handleInternal(RoutingContext context,
{
RoutingContextUtils
.getAsFuture(context, SC_RESTORE_JOB)
- .compose(job -> {
- if (job.status.isFinal())
+ .compose(existingJob -> {
+ if (existingJob.status.isFinal())
{
// skip the update, since the job is in the final state already
- logger.debug("The job has completed already. job={}", job);
+ logger.debug("The job has completed already. job={}",
existingJob);
return
Future.failedFuture(wrapHttpException(HttpResponseStatus.CONFLICT,
- "Job is already
in final state: " + job.status));
+ "Job is already
in final state: " + existingJob.status));
}
return executorPools.service()
- .executeBlocking(() ->
restoreJobDatabaseAccessor.update(requestPayload, job.jobId));
- })
- .onSuccess(job -> {
- logger.info("Successfully updated restore job. job={}, request={},
remoteAddress={}, instance={}",
- job, requestPayload, remoteAddress, host);
- if (job.status == RestoreJobStatus.SUCCEEDED)
- {
- metrics.successfulJobs.metric.update(1);
- long startMillis = UUIDs.unixTimestamp(job.jobId);
- long durationMillis = System.currentTimeMillis() - startMillis;
- // toNanos does not overflow. Nanos in `long` can at most
represent 106,751 days.
- metrics.jobCompletionTime.metric.update(durationMillis,
TimeUnit.MILLISECONDS);
- }
+ .executeBlocking(() ->
restoreJobDatabaseAccessor.update(requestPayload, existingJob.jobId))
+ .onSuccess(updatedJob -> {
+ logger.info("Successfully updated restore
job. job={}, request={}, remoteAddress={}, instance={}",
+ updatedJob, requestPayload,
remoteAddress, host);
+ if (updatedJob.status ==
RestoreJobStatus.SUCCEEDED)
+ {
+
metrics.successfulJobs.metric.update(1);
+ long startMillis =
UUIDs.unixTimestamp(updatedJob.jobId);
+ long durationMillis =
System.currentTimeMillis() - startMillis;
+ // toNanos does not overflow. Nanos in
`long` can at most represent 106,751 days.
+
metrics.jobCompletionTime.metric.update(durationMillis, TimeUnit.MILLISECONDS);
+ }
- if (job.secrets != null)
- {
- metrics.tokenRefreshed.metric.update(1);
- }
+ if (updatedJob.secrets != null)
+ {
+
metrics.tokenRefreshed.metric.update(1);
+ }
-
context.response().setStatusCode(HttpResponseStatus.OK.code()).end();
+
context.response().setStatusCode(HttpResponseStatus.OK.code()).end();
+ // Fire-and-forget on a worker thread —
notifying the restore system should not
+ // block the event loop or delay the HTTP
response.
+ executorPools.service()
+ .runBlocking(() ->
notifyPhaseSignalMaybe(existingJob, updatedJob.status));
+ });
Review Comment:
Done in the latest commit- handler is also cleaner now since it no longer
has to rebuild the full job after the update.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]