-
Notifications
You must be signed in to change notification settings - Fork 500
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
500 error (OptimisticLockException) when uploading multiple tabular files #11265
Comments
FWIW: Without checking all the details, I think it ~makes sense if the files are being added separately. After the upload of an ingestable file, there should be an ingest lock. If that's ignored, it's possible that the ingest could finish and update the dataset, causing other updates (like new file adds) to fail since they are working on an outdated copy of the dataset. My guess is that, as with #10793, we aren't handling it well. Probably should either be checking locks earlier and/or catching the 500 to give a better error. With that, you'd still have to retry uploads. A bigger refactor, along the lines of #10856 might make it possible for ingest to not update the overall version and therefore not cause an optimistic lock exception. A work-around would be to check for locks and wait until they are released before adding more files, or using the multiple file add (available for direct upload via pyDvuploader, not sure if pyDataverse can do it.). |
I thought python-dvuploader v0.2.3 was in fact using @kmika11 I would absolutely recommend to try to use an implementation of direct upload that DOES take advantage of the batch update, for all sorts of other reasons outside of this specific problem. |
I just edited my comment above - I apparently misread the opening description; if it's bombing reproducibly in both places, demo and prod., that's less weirdness to worry about. So, 🎉 . |
@JR-1991 thanks. Please note that I was replying to @qqmyers' comment, where he suggested that finalizing one file at a time may have been the issue. Your implementation of direct upload appears to be doing the right thing, just as Jim and I assumed. I.e., using
@qqmyers was your comment written under the assumption that direct upload was NOT enabled for the dataset? - because it IS enabled for the dataset above (or did I miss something else) ... Oh, wait - are we running into the optimistic lock despite the fact that multi-file add is used; simply because the file in question is so tiny, it ingests before the rest of |
From the stack trace, it looks like the single file version is getting called, but it is direct upload. The json is getting printed per file, I think by
|
Hmmm. I'm not seeing the single-file |
... that said, I'm not seeing the words |
That json print in the stacktrace may be from here: dataverse/src/main/java/edu/harvard/iq/dataverse/datasetutility/OptionalFileParams.java Line 335 in d3cba3f
and OptionalFileParams is created from json for each file from inside dataverse/src/main/java/edu/harvard/iq/dataverse/datasetutility/AddReplaceFileHelper.java Line 2044 in d3cba3f
I think |
Ah - you're right. The logging in Datasets is set to fine at
That said, the msgt in OptionalFileParams (which is just System.out.println) would get called for the single or multiple file api calls, and ingest is only started after the UpdateDatasetVersionCommand in the addFiles code - at dataverse/src/main/java/edu/harvard/iq/dataverse/datasetutility/AddReplaceFileHelper.java Line 2163 in d3cba3f
|
I'm not really working today; would need to take a closer look at the sequence of events there. But I think this line in the stacktrace suggests that it was an
What is the database transaction here - is it the execution of the UpdateDatasetVersionCommand? Or is the entire API call executed in one transaction? If it is the latter, I can see how an ingest of 3 really tiny files can beat the command, and result in an optimistic lock, maybe? |
Makes sense. The command only supports a transaction so it wouldn't create a new one if one was already started. |
@kmika11 The preliminary investigation appears to suggest that this is NOT the result of anything in your python code, or anything wrong with the python implementation of direct upload that you are using. What seems to be causing this is just the fact that the 3 ingestable files in the batch are so microscopically small; because of that they ingest too fast for their own good, in a way that interferes with the main update (the "registering files" step). This is objectively a bug in Dataverse; and it should be possible to prevent this from happening, on our end. In the more immediate terms, there should be some quick workarounds that would allow you to upload these files. Operating under the assumption that you probably don't need these files to be ingested as tabular data (somewhat unlikely that any useful tabular metadata can be extracted from a 34 byte-sized csv file - ?): The simplest way to avoid having these files ingested is to rename them .txt instead of .csv. (and/or change the mime type to I may have some questions about how the notebook is handling the uploads (the use of replace specifically); I may ask separately. @JR-1991 It is not explicitly mentioned in the README.md, but can I pass the |
@landreev, the Here’s the error message I get when ingestion is enabled:
|
@landreev I can't speak to the use of replace as its identified in the error message here. I'm using easyDataverse and its implementation of dvuploader & direct upload for uploading files. My code doesn't build the url to use I do not explicitly identify the mime type because there isn't a mimetype argument in A final note - the files appear to upload fine, and do get correctly assigned to the right dataset. And the dataset does not appear locked when I review it in a browser. I just get a 500 error that interrupts multiple dataset deposits that are looped. |
@kmika11 easyDataverse utilizes python-dvuploader on the backend and indeed the After the PR is approved, I will launch the new version and update easyDataverse accordingly 🙌 |
Since Jan is adding support for the |
@kmika11 @JR-1991
Why is this
|
@landreev there is a mechanism to check whether a file already exists and differs in hash. If so, the uploader flags it as I noticed that the request is sent regardless of the payload content, including empty cases, which is definitely a bug. I have opened an issue regarding this and will add this to the
Edit: Added the parts of code to the PR and tested both with replacement and novel files. Now works as expected. |
@JR-1991 Thank you so much for addressing these issues so quickly! |
@landreev Thanks too for the hints! Helps a lot to debug the uploader 😊
Yes, according to the code, an empty list should be sent when the files are entirely new in the dataset.
Perfect! The PR includes a condition not to send anything if the list is empty. Thus, we can work around this in the meantime. |
FWIW: If I'm following correctly - sounds like the empty replace can be fixed on both sides, but if the replace call is ever not empty, you may need to wait or would risk an OptimisticLockException from indexing, ingest, etc. in the /addFiles call. |
@qqmyers good point, I will add a lock-check to be safe. |
Correct. In this instance it was caused by an ingest that was too fast conflicting with an empty /replace. But it should be entirely possible to run into this with a longer-running ingest or indexing and a similarly expensive /replace, they just need to overlap in a certain way. |
With @JR-1991 and I looking on, @kmika11 was able to reproduce the following issue on https://demo.dataverse.org running Dataverse 6.5 (technically
"version":"6.5","build":"iqss-3"
) but she's having the same problem on Harvard Dataverse (which runs the same version).Katie is using the latest release of python-dvuploader: v0.2.3.
Her notebook is here: https://github.com/kmika11/harvard-college-observatory-announcements/blob/32a0b9aa287eb20f0aeef5853ffec2bb1ec95131/curation_script_HCO.ipynb
The script creates a dataset ( https://demo.dataverse.org/dataset.xhtml?persistentId=doi:10.70122/FK2/0HMSY2&version=DRAFT ) with a six files, three of which are tabular, before it unexpectedly stops uploading the remaining files from the list of 60 total files:
Here's a screenshot of the error Katie sees in here notebook:
Expand the section below to see the traceback in the notebook.
Notebook traceback
On the backend, we get a 500 error because of an OptimisticLockException:
jakarta.ejb.EJBException: Exception [EclipseLink-5010] (Eclipse Persistence Services - 4.0.1.payara-p2.v202310250827): org.eclipse.persistence.exceptions.OptimisticLockException Exception Description: The object [[DatasetVersion id:283666]] cannot be merged because it has changed or been deleted since it was last read. Class> edu.harvard.iq.dataverse.DatasetVersion
The full OptimisticLockException and surrounding stacktrace is below.
500 error (OptimisticLockException)
The text was updated successfully, but these errors were encountered: