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

Feature/oaipmh without dep max modified #509

Merged
merged 17 commits into from
Jan 29, 2020

Conversation

jannistsiroyannis
Copy link
Contributor

This prepares for removing the depMaxModified column, as it removes its only use (which is in OAIPMH).
It does not (yet) remove the updating of depMaxModified (which is the real win). It only removes the need for it.

Copy link
Contributor

@olovy olovy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

The only problem I see (which existed before) is that the request holds a database connection while calling into loadEmbellished, followDependers, getAttachedHoldings etc. risking connection grabbing deadlocks. (this is because of two changes: setting connectionpool size to a small value and infinite connection timeouts).

The only sane solution I see is to have a separate connection pool for the API? (initialized by PostGreSQLComponent). We could do this as a separate feature branch.


public class Helpers
{
public static class ResultIterator implements Iterator<Document>, AutoCloseable {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

for (Tuple2<String, String> depender : dependers)
{
String dependerId = depender.getFirst();
Document dependerDocument = OaiPmh.s_whelk.loadEmbellished(dependerId);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be getDocument()? (embellish is done in ResponseCommon.emitRecord())

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Fixing it!

@olovy olovy merged commit 15658ef into develop Jan 29, 2020
@olovy olovy deleted the feature/oaipmh-without-depMaxModified branch January 29, 2020 16:02
olovy pushed a commit that referenced this pull request Feb 3, 2020
* Make emitRecord take Documents instead of sql resultsets.

* Make both from and until optional again.

* Make the adhoc tests at least possibly reusable.
olovy added a commit that referenced this pull request Feb 3, 2020
* Make emitRecord take Documents instead of sql resultsets.

* Make both from and until optional again.

* Make the adhoc tests at least possibly reusable.

Co-authored-by: Jannis Mohlin Tsiroyannis <[email protected]>
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.

2 participants