Store async upload job in the request's thread #1407

IvanBorislavovDimitrov · 2023-11-24T10:32:44Z

No description provided.

radito3 · 2023-11-24T11:59:25Z

...eb/src/main/java/org/cloudfoundry/multiapps/controller/web/api/impl/FilesApiServiceImpl.java

+                            .id(entry.getId())
+                            .delete();
+        } catch (Exception e) {
+            LOGGER.error(Messages.ERROR_OCCURRED_WHILE_DELETING_JOB_ENTRY, e);


Apart from logging the error, should we handle this case? If a job fails to be submitted but also fails to be deleted from the DB, this would lead to a resource leak.
Fetching the job would always return 200 with state "initial" but it will never run, so a client would hang indefinitely.

...ler-api/src/main/java/org/cloudfoundry/multiapps/controller/api/model/AsyncUploadResult.java

...in/resources/org/cloudfoundry/multiapps/controller/persistence/db/changelog/db-changelog.xml

...eb/src/main/java/org/cloudfoundry/multiapps/controller/web/api/impl/FilesApiServiceImpl.java

theghost5800 · 2023-12-08T15:18:16Z

...eb/src/main/java/org/cloudfoundry/multiapps/controller/web/api/impl/FilesApiServiceImpl.java

+            if (runningTasks.get(existingJob.getId()) != null) {
+                LOGGER.info(Messages.ASYNC_UPLOAD_JOB_EXISTS, urlWithoutUserInfo, existingJob);
+                return ResponseEntity.status(HttpStatus.SEE_OTHER)
+                                     .header(HttpHeaders.LOCATION, getLocationHeader(spaceGuid, existingJob.getId()))
+                                     .header("x-cf-app-instance", configuration.getApplicationGuid() + ":" + existingJob.getInstanceIndex())
+                                     .build();
+            } else {
+                LOGGER.warn(Messages.THE_JOB_EXISTS_BUT_IT_IS_NOT_RUNNING_DELETING);
+                deleteAsyncJobEntry(existingJob);
+            }


Extract in a new method

Extracted another part of the method, because here it returns

theghost5800 · 2023-12-08T15:34:46Z

...eb/src/main/java/org/cloudfoundry/multiapps/controller/web/api/impl/FilesApiServiceImpl.java

+                                                                   .bytes(count.get())
+                                                                   .build());
+            }
+            var jobWithLatestState = getJob(job.getId(), spaceGuid, namespace);


Sonarcloud reports that jobWithLatestState can be null. I had noticed that at the beginning AsyncUploadJobEntry is loaded from the db, so we can reuse the object instead of calling db twice. What is the possibility to have different state of AsyncUploadJobEntry during several checks which does not perform any external calls to other services?

The idea is to fetch the new state of the object from the database as it should be changed because the Future task is done. This covers the race condition between updating/getting the job
I do not think the variable can be null, we've already checked it

theghost5800 · 2023-12-08T15:45:11Z

...eb/src/main/java/org/cloudfoundry/multiapps/controller/web/api/impl/FilesApiServiceImpl.java

@@ -233,20 +268,29 @@ private AsyncUploadJobEntry getExistingJob(String spaceGuid, String namespace, S
                                   .user(SecurityContextUtil.getUsername())
                                   .namespace(namespace)
                                   .url(url)
+                                   .instanceIndex(configuration.getApplicationInstanceIndex())


What will happen during bg update of deploy-service and both versions are up and running? Then app instance index will match but job won't exist on one of the app instances.

I think the request will fail because of the x-cf-app-instance, since it points to the application that we're stopping which will retry the upload. When both run it will call the right one

Yes, tested it with simulated bg-deploy of the deploy-service.
When 2 apps are running it still sends requests to the one that the upload started because of x-cf-app-instance
When the old one is stopped the router returns 400 which retries the whole upload and succeeds, 400 is returned again because of x-cf-app-instance

theghost5800 · 2023-12-08T15:57:08Z

multiapps-controller-web/src/main/java/org/cloudfoundry/multiapps/controller/web/Messages.java

+    public static final String JOB_0_WAS_NOT_FOUND_IN_THE_RUNNING_TASKS = "Job \"{0}\" was not found in the running tasks";
+    public static final String JOB_IS_NOT_BEING_EXECUTED = "Job is not being executed.";
+    public static final String JOB_0_EXISTS_IN_STATE_1_BUT_DOES_NOT_EXISTS_IN_THE_RUNNING_TASKS = "Job \"{0}\" exists in state \"{1}\" but does not exists in the running tasks";
+    public static final String JOB_THREAD_IS_NOT_RUNNING_BUT_STATE_IS_STILL_IN_PROGRESS_UPLOAD_FAILED = "Job thread is not running but state is still in progress. Upload failed...";


I think we are using multiple dots only when something continue to run on background. I think this is not the case here, it will retry only if client retry http request, right?