Skip to content
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

Fix postgres unique constraint exception #12373

Merged
merged 3 commits into from
Jan 17, 2025
Merged

Fix postgres unique constraint exception #12373

merged 3 commits into from
Jan 17, 2025

Conversation

LoayGhreeb
Copy link
Member

@LoayGhreeb LoayGhreeb commented Jan 9, 2025

Attempt to fix #12167.

I cannot reproduce the issue on my machine, but I guess it may be related to background threads and quickly updating the entry.
One possible scenario is that the user is updating the field quickly, and updating the field is a critical section that should be completed as an atomic operation without interruptions.

updateEntry(BibEntry entry, Field field) {
	removeField(entry, field);
	insertField(entry, field);
}

The issue might occur if the first thread removes the field, but before it inserts the field again, another thread updates the same entry and also removes the field. This could result in both threads attempting to insert the same field into the table twice, violating the primary key constraint (entryId, fieldName), which must be unique.

Mandatory checks

  • I own the copyright of the code submitted and I licence it under the MIT license
  • Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

@LoayGhreeb
Copy link
Member Author

@ytzemih, can you test the build of this PR and check if it resolves the issue? I cannot reproduce the issue on my machine, so it's a bit difficult to debug or see the results.

@ytzemih
Copy link

ytzemih commented Jan 11, 2025

@LoayGhreeb Thanks for the candidate fix. Would you be able to rebuild the .deb in order for the new deb config to be applied? See #12370
Then, I'm happy to try it out. Thanks a lot.

@LoayGhreeb
Copy link
Member Author

Would you be able to rebuild the .deb in order for the new deb config to be applied? See #12370

Alright, I just updated the branch. The build will be available in 10 minutes.

@ytzemih
Copy link

ytzemih commented Jan 12, 2025

@LoayGhreeb Thanks for trying. For some reason, #12370 is still not fixed for my Linux. I can't install JR and I'm not keen on starting to backport/fiddle around with incompatible ALSA libraries. I hope, @Siedlerchr and I can figure that out soon and then I'll try again.

Copy link
Contributor

github-actions bot commented Jan 12, 2025

The build for this PR is no longer available. Please visit https://builds.jabref.org/main/ for the latest build.

@Siedlerchr
Copy link
Member

@ytzemih there were missing a few lines in the yaml that needed to be adapted so no new build was produced. Should work now with build date 12.01.2025

@ytzemih
Copy link

ytzemih commented Jan 14, 2025

@LoayGhreeb and @Siedlerchr, after a few days of working with that PR, neither the unique constraint exception (#12167) nor of the problems with the PostgreSQL (#12190) seem to show up. I'll be having an eye on this issue in the coming days, but it seems that this could be a fix.

@ytzemih
Copy link

ytzemih commented Jan 16, 2025

Ok, I think this issue is gone. I can't seem to get the unique constraint violations anymore. However, my hope about #12190 being fixed as well has gone. The latter still occurs. But I'll add further comments in the corresponding places.

@Siedlerchr
Copy link
Member

@LoayGhreeb I think we should merge this

@LoayGhreeb
Copy link
Member Author

Nice to hear it's fixed now. Thanks for the feedback, @ytzemih!. I'll merge this PR.

For the #12190, I will investigate it and try to submit a PR as soon as possible.

@LoayGhreeb LoayGhreeb enabled auto-merge January 17, 2025 20:45
@LoayGhreeb LoayGhreeb requested a review from Siedlerchr January 17, 2025 20:46
@LoayGhreeb LoayGhreeb added this pull request to the merge queue Jan 17, 2025
Merged via the queue into main with commit 3caa658 Jan 17, 2025
26 checks passed
@LoayGhreeb LoayGhreeb deleted the fix-db-sync branch January 17, 2025 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PostgreSQL unique constraint violation when editing an entry
3 participants