SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry

Anytime I run s3fs I run into a duplicate key error.

Do you mean any time you run a Cache Refresh or do you mean any time s3fs is enabled?

I’m a bit hesitant to just switch this to an upsert without more data, in the past I have encountered issues with endless loops accidentally occurring and having an upsert here could hide them if they ever came back and may also end up with the wrong data being inserted.

This portion of the code has a long standing comment about being concerned about integrity constraints being possible. I’ve not been able to think of a good method around the concern. It is known it can occur if your bucket has an object named “/path/object” and an object named “/path/object/another object”. While this is strictly valid in S3 it is an almost impossible situation on standard file systems and finding a way to make them coexist doesn’t seem practical. (Though in 4.x maybe we could make the “dir” field part of the primary key to avoid collision. ) Additionally while it’s not commented as a warning in the code it’s possible an old folder entry in the main s3fs table now conflicts (could truncate the table but would not recommend that without testing in a dev lab first).

I’m also remembering one other issue we can’t fix in 3.x that may cause it. #2833951: Issue with directories and trailing whitespaces

Any chance you could put this in a lab environment against your bucket with a debugger to confirm what the stack looks like? I know with a bucket that large it isn’t the easiest process.

Well you managed to fire off an idea in my head about how to handle this (sectioning off the data).

This is VERY rough code and only given very basic testing (I made an intentional dupe to confirm it works as I expect in my local lab) however it might help in finding out exactly what the cause is for you and maybe work as a framework for a long term solution.

Not necessarily up to coding standards or best practices, it isn't pretty, and it could take a while in slow environments as it is falling back to writing SINGLE records at a time for any batch that had an entry that failed with an integrity constraint and than logging it into the Drupal logs.

This is assuming it is in the FOLDER section and not the FILES section of the code since that is where you started with on your patch.

@b_sharpe

Were you able to determine what record(s) were causing your issue and how they fit in to your file structure?

Overall I believe the initial version of the patch I posted seems like a good idea.

I've cleaned it up to use the already injected messenger service and added a section to the README.txt to be referenced on how to solve the two causes that I can think of.

This still does have the concern of making a large number of round trip calls to SQL, however I believe outside of excessive network latency this should not cause too big a problem.

Ignored the possibility of an IntegrityConstraintViolationException on the file section as if any errors occur we have a much more significant problem in the code.

Based on how S3fs functions this should allow the refresh to compete and the majority of operations should be fine after the refresh. We would expect isdir() would fail however direct access to the file by path should succeed as stat() will return the object allowing it. Eventually though the file record could get replaced by a directory record causing possible data-loss if allowed to continue operating.

Pushed patch in #8 to dev, Moving to postponed to allow more time for feedback on what the root cause was so we can attempt to determine if there is another issue we need to solve. At least with this patch we can allow the sync to continue and provide feedback to admins.

Appreciate you following up so that we can know for sure there wasn’t a deeper fault in the refresh code being hidden here.

The only thing I might suggest here is that we allow also log the issues so you can see them outside of the window/drush command.

At the moment I’ll admit this is a bit of a compromise on user experience. We don’t currently have a logging service injected to this area of code, we’re in beta so I am trying to avoid disrupting the API any more by making changes unless necessary, and I’m trying to avoid adding any more \Drupal:: calls unless necessary as part of the process of cleaning up the code and better adhering to Drupal code standards.

I can add the logging to improve the user experience when we pull this forward into the 4.x rewrite.

This made it into the in progress 4.x work issue fork [#3206162-4.x-version] with the logging improvements so I'm going to close it out.