Improved handling of Globus uploads (experimental async framework) #10781

landreev · 2024-08-19T14:50:47Z

What this PR does / why we need it:

It solves the 2 problems described in the linked issue:

An api call to /addFiles via curl has been replaced with calling the corresponding method in AddReplaceFileHelper. In addition to the problems with the curl implementation already described in the issue, the old code was not making any attempt to parse the output of the api; so if /addFiles dropped one or more files from the submitted entries, for whatever reason, the Globus service method was still registering that as a complete success. The new code checks the results in a more careful way.
A new implementation of the upload transfer polling solves the issue where the Dataverse instance needs to stay up and the same service method needs to loop continuously for the duration of the transfer. The new framework relies on a schedule-run execution of the polling calls and saving the state of the ongoing tasks in the database. This gives an extra advantage of giving an admin a relatively easy way to add the files to the user's dataset after the fact in a scenario where the remote Globus transfer succeeded, but the files failed to be added to the dataset automatically, for whatever reason. However, due to the experimental nature of this new framework, I'm leaving it not enabled by default. Pending gaining more evidence on how it holds up in a prod.-like environment.

The handling of diagnostics and notifications have generally been improved in the PR. Among other things, there are now explicit failure notifications that are sent to the users when uploads fail completely. The existing implementation only logs such events without notifying the user.

Which issue(s) this PR closes:

Closes Globus integration: further improvements/refactoring #10623

Special notes for your reviewer:

Note that I don't use the EJB timers for the scheduled execution of the task polling. I used the jakarta.enterprise.concurrent.ManagedScheduledExecutorService instead which, from what I understand, is a preferred way now.

Suggestions on how to test this:

In order to test this PR, and other Globus-related things going forward, I added a Globus setup to dataverse-internal yesterday (Globus-enabled storage configuration plus the upload/download web app). This volume:

Is connected to remote storage at NESE that was set up for us specifically for testing uploads and downloads. This setup is identical to what was previously configured on demo.dataverse.org. I haven't tested it much myself though.

The instructions on how to upload data via Globus in Datataverse can be found here: https://github.com/IQSS/dataverse.harvard.edu/blob/master/doc/globus/upload.md.

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

Is there a release notes update needed for this change?:

Additional documentation:

Preview at https://dataverse-guide--10781.org.readthedocs.build/en/10781/installation/config.html#dataverse-globus-taskmonitoringserver

…obus-uploaded files #10623

…taverse-accessible. #10623

… some cleanup and refinements. #10623

github-actions · 2024-08-19T14:51:30Z

src/main/java/edu/harvard/iq/dataverse/ExternalFileUploadInProgress.java

+ * externally. (?)
+ */
+@NamedQueries({
+	@NamedQuery( name="ExternalFileUploadInProgress.deleteByTaskId",


🚫 [reviewdog] <com.puppycrawl.tools.checkstyle.checks.whitespace.FileTabCharacterCheck> _{reported by reviewdog 🐶}
File contains tab characters (this is the first instance).

coveralls · 2024-08-19T14:57:15Z

coverage: 20.681% (-0.05%) from 20.734%
when pulling 5dc386f on 10623-globus-improvements
into 8f3fc4a on develop.

landreev · 2024-08-20T14:51:41Z

I'm not sure what's going on with Jenkins/integration tests. But I'll figure that out separately. I'm going to go ahead and un-draft the PR...

src/main/java/edu/harvard/iq/dataverse/engine/command/impl/UpdateDatasetVersionCommand.java

qqmyers

Nice! Looks good to me. I made some comments/responded to notes in the code, but other than any open @todos and some minor cleanup, it looks ready for QA to me. Goodbye to exec-ing curl!

I'll put request changes to avoid it switching columns until @landreev is ready, but probably no need for another review of changes.

src/main/java/edu/harvard/iq/dataverse/ExternalFileUploadInProgress.java

src/main/java/edu/harvard/iq/dataverse/api/Datasets.java

qqmyers · 2024-08-20T16:49:04Z

src/main/java/edu/harvard/iq/dataverse/api/Datasets.java

@@ -4034,6 +4035,8 @@ public Response addGlobusFilesToDataset(@Context ContainerRequestContext crc,
            return wr.getResponse();
        }

+        // @todo check if the dataset is already locked!


Makes sense - could there be a constraint when trying to create a lock, e.g. only one lock of a given time per dataset rather than a separate check?

You meant "of a given type", right? The addDatasetLock() already has a check for an existing lock of the same type (then it will just return the existing lock instead of creating a new one).
As for implementing this as a hard constraint - are we positive we'll never want multiple locks of the same type - some workflows maybe?
I addressed this specific situation with a separate lock check. We don't check for locks consistently in our APIs, I'm assuming under the assumption that it will be done when the the relevant commands are executed. But in this specific case, there is a potential for doing a huge amount of work before that UpdateDatasetVersionCommand is called in the end.

src/main/java/edu/harvard/iq/dataverse/api/ApiConstants.java

src/main/java/edu/harvard/iq/dataverse/engine/command/impl/UpdateDatasetVersionCommand.java

src/main/java/edu/harvard/iq/dataverse/globus/TaskMonitoringServiceBean.java

src/main/java/edu/harvard/iq/dataverse/ingest/IngestServiceBean.java

qqmyers · 2024-08-20T18:10:09Z

src/main/java/edu/harvard/iq/dataverse/timer/DataverseTimerServiceBean.java

        } catch (UnknownHostException ex) {
            Logger.getLogger(DataverseTimerServiceBean.class.getName()).log(Level.SEVERE, null, ex);
        }

        if (timer.getInfo() instanceof MotherTimerInfo) {
-            logger.info("Behold! I am the Master Timer, king of all timers! I'm here to create all the lesser timers!");
+            logger.fine("Behold! I am the Master Timer, king of all timers! I'm here to create all the lesser timers!");


I'm going to miss these :-(

You can still have it - at a low price of one FINE logging setting.

Ha! It's like an old friend at this point. 😄

src/main/java/edu/harvard/iq/dataverse/util/SystemConfig.java

src/main/java/propertyFiles/Bundle.properties

landreev · 2024-09-05T20:35:13Z

I resolved the merge conflicts yesterday. Also added some info under "How to Test".

Resolved conflicts: src/main/java/edu/harvard/iq/dataverse/api/Datasets.java (#10623)

landreev · 2024-09-10T15:16:18Z

Resolved another merge conflict.

qqmyers

Looks like my comments have been addressed. I still see the review dog complaint about tab chars in ExternalFileUploadInProgress - can that be fixed so we avoid the style fail?

pdurbin · 2024-09-23T19:33:31Z

@qqmyers good catch. I put in a commit to quiet down that yappy dog: 7b6f81e

I also made a small doc tweak: 2baf62e

Then I tested on internal. With the new flag off I was able to upload file. ✅

With the flag on, Globus is telling me "transfer complete" but the file is not in the dataset and the dataset has the "Globus Transfer in Progress" lock. ❌

@landreev is going to take a look.

Conflicts: doc/sphinx-guides/source/installation/config.rst

…ataverse into 10623-globus-improvements

github-actions · 2024-09-25T15:54:45Z

📦 Pushed preview images as

ghcr.io/gdcc/dataverse:10623-globus-improvements

ghcr.io/gdcc/configbaker:10623-globus-improvements

🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name.

pdurbin · 2024-09-25T16:02:07Z

Turned out to be a doc problem fixed now. Works great! Especially for an experimental feature. 😜

Also, API tests are passing as of the last code change: https://jenkins.dataverse.org/job/IQSS-Dataverse-Develop-PR/job/PR-10781/7/testReport/

Merging.

landreev added 9 commits August 5, 2024 17:17

a quick experimental AddReplaceFileHelper implementation of adding Gl…

65ec69f

…obus-uploaded files #10623

no need to try to calculate checksums if this globus storage isn't da…

0495160

…taverse-accessible. #10623

more globus mods (work in progress). #10623

ba66138

new class files that weren't included in the last commit #10623

dac5302

fixing some bad changes that got committed earlier #10623

4080341

cleanup #10623

e086a60

more testing/debugging #10623

35ce7ef

this is a working, but still work-in-progress state of things - needs…

d4b9bac

… some cleanup and refinements. #10623

refined logging #10623

9c62b81

github-actions bot reviewed Aug 19, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

Added notifications for various failure cases. #10623

8cdff8d

This comment has been minimized.

Sign in to view

Config guide entry. #10623

531e25c

This comment has been minimized.

Sign in to view

Added a few more doc notes. #10623

007d715

This comment has been minimized.

Sign in to view

typo #10623

6fcb285

landreev marked this pull request as ready for review August 20, 2024 14:51

This comment has been minimized.

Sign in to view

qqmyers self-requested a review August 20, 2024 15:04

qqmyers self-assigned this Aug 20, 2024

cut-and-paste error #10623

f6882df

landreev commented Aug 20, 2024

View reviewed changes

src/main/java/edu/harvard/iq/dataverse/engine/command/impl/UpdateDatasetVersionCommand.java Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

(#10623)

4ae3ee6

This comment has been minimized.

Sign in to view

qqmyers requested changes Aug 20, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

landreev added the Size: 30 A percentage of a sprint. 21 hours. (formerly size:33) label Sep 5, 2024

Merge branch 'develop' into 10623-globus-improvements

5dc386f

Resolved conflicts: src/main/java/edu/harvard/iq/dataverse/api/Datasets.java (#10623)

This comment has been minimized.

Sign in to view

cmbz added the FY25 Sprint 6 FY25 Sprint 6 label Sep 11, 2024

cmbz mentioned this pull request Sep 11, 2024

Epic: GREI 5 - Use Cases IQSS/dataverse-pm#119

Open

9 tasks

pdurbin self-assigned this Sep 23, 2024

qqmyers approved these changes Sep 23, 2024

View reviewed changes

pdurbin added 3 commits September 23, 2024 11:21

remove tabs to make reviewdog happy. woof! #10623

7b6f81e

globus doc tweaks #10623

2baf62e

Merge branch 'develop' into 10623-globus-improvements #10623

5d8c760

This comment has been minimized.

Sign in to view

pdurbin assigned landreev Sep 23, 2024

Merge branch 'develop' into 10623-globus-improvements

6c5194f

Conflicts: doc/sphinx-guides/source/installation/config.rst

This comment has been minimized.

Sign in to view

landreev added 2 commits September 25, 2024 10:20

Updated the docs to reflect the new name of a JVM option (#10623)

d610094

Merge branch '10623-globus-improvements' of https://github.com/IQSS/d…

bf91ce2

…ataverse into 10623-globus-improvements

This comment has been minimized.

Sign in to view

improve release note #10623

682c89f

pdurbin changed the title ~~Improved handling of Globus uploads~~ Improved handling of Globus uploads (experimental async framework) Sep 25, 2024

pdurbin merged commit d40ce32 into develop Sep 25, 2024
13 of 14 checks passed

pdurbin deleted the 10623-globus-improvements branch September 25, 2024 16:02

pdurbin added this to the 6.4 milestone Sep 25, 2024

pdurbin unassigned pdurbin and landreev Sep 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improved handling of Globus uploads (experimental async framework) #10781

Improved handling of Globus uploads (experimental async framework) #10781

landreev commented Aug 19, 2024 •

edited by pdurbin

Loading

github-actions bot Aug 19, 2024

coveralls commented Aug 19, 2024 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

landreev commented Aug 20, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

qqmyers left a comment

qqmyers Aug 20, 2024

landreev Aug 23, 2024

qqmyers Aug 20, 2024

landreev Aug 22, 2024

pdurbin Sep 10, 2024

This comment has been minimized.

landreev commented Sep 5, 2024

landreev commented Sep 10, 2024

This comment has been minimized.

qqmyers left a comment

This comment has been minimized.

pdurbin commented Sep 23, 2024

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Sep 25, 2024

pdurbin commented Sep 25, 2024

Improved handling of Globus uploads (experimental async framework) #10781

Improved handling of Globus uploads (experimental async framework) #10781

Conversation

landreev commented Aug 19, 2024 • edited by pdurbin Loading

github-actions bot Aug 19, 2024

Choose a reason for hiding this comment

coveralls commented Aug 19, 2024 • edited Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

landreev commented Aug 20, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

qqmyers left a comment

Choose a reason for hiding this comment

qqmyers Aug 20, 2024

Choose a reason for hiding this comment

landreev Aug 23, 2024

Choose a reason for hiding this comment

qqmyers Aug 20, 2024

Choose a reason for hiding this comment

landreev Aug 22, 2024

Choose a reason for hiding this comment

pdurbin Sep 10, 2024

Choose a reason for hiding this comment

This comment has been minimized.

landreev commented Sep 5, 2024

landreev commented Sep 10, 2024

This comment has been minimized.

qqmyers left a comment

Choose a reason for hiding this comment

This comment has been minimized.

pdurbin commented Sep 23, 2024

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Sep 25, 2024

pdurbin commented Sep 25, 2024

landreev commented Aug 19, 2024 •

edited by pdurbin

Loading

coveralls commented Aug 19, 2024 •

edited

Loading