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.

Sign up for GitHub

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

Jump to bottom

New Cottontail API, PolyphenyDB #224

Merged
merged 119 commits into from
Feb 7, 2022
Merged

New Cottontail API, PolyphenyDB #224

merged 119 commits into from
Feb 7, 2022

Conversation

silvanheller
Copy link
Member

@silvanheller silvanheller commented Oct 18, 2021 •
edited
Loading

Major Changes

  • Updated Cottontail DB connector to version 0.13.x
  • Added Polypheny DB connector (experimental)
  • Deprecated ADAMpro connector
  • Refactored DB integration tests to me more in line with Cineast API
  • Changed connection management in Cineast (which is going to be part of a future discussion)

Breaking

  • Versions of Cottontail DB older than 0.13.0 are no longer supported.

Ralph Gasser and others added 30 commits August 25, 2021 17:17
Started implementation of Polypheny DB adapter.
6700870
Added missing library.
bf301b2
Started refactoring of data sources and configuration thereof.
5b630a2
@ppanopticon
Added Polypheny-DB dependency as actual Maven reference.
201fa6d
@ppanopticon
Fixed PolyphenyEntityCreator
73ef482
@ppanopticon
Added missing comment.
c2872da
@ppanopticon
Added LOGGER field to PersistencyWriter
aaed7c9
@ppanopticon
CottontailWriter no longer closes the CottontailWrapper.
8223fd8
@ppanopticon
Implemented first draft of PolyphenyWriter
211947d
@ppanopticon
Fixed few issues with regards to INSERTS into Polypheny DB.
60e86ff
@ppanopticon
Moved LOGGER field to DBSelector implementation.
d3e1236
@ppanopticon
Fixed minor bugs.
15930cb
@ppanopticon
Simplified array construction in PolyphenyWriter.
e2ecbda
@ppanopticon
Started implementation of PolyphenySelector.
58b65ed
@ppanopticon
Completed implementation of PolyphenySelector
023992b
CottontailWrapper now keeps its ManagedChannels in a pool.
ce2bb0e
@ppanopticon
Made necessary adjustments to Cottontail DB client (0.13.0)
46fdb04
@ppanopticon
Removed superfluous import.
de37311
@ppanopticon
Merge remote-tracking branch 'origin/polyphenydb' into polyphenydb
ac2a29a 
# Conflicts:
#	cineast-core/src/main/java/org/vitrivr/cineast/core/db/cottontaildb/CottontailSelector.java
Fixed tow minor issues.
1c294a9
Fixed an issue in CottontailSelector.getNearestNeighbourRows().
a3b1340
Added implementation for getNearestNeighborGeneric().
03d2af4
@ppanopticon
Fixed issues in the way queries NNS queries were formed.
c76e494
@ppanopticon
Adjusted log levels.
20fe4c5
Fixed and optimized PolyphenySelector
30e0b83
@ppanopticon
Made adjustments to support recent API changes.
63b7b74
Merged dev.
cb11a15
Fixed LDC21TemporalUpdateCommand.
35d9490
@sauterl
Fixed broken LSC Temporal Update
6f935eb
@ppanopticon
Added simple CLI command to benchmark Polypheny DB.
bf2bd49
@ppanopticon
Copy link
Member

ppanopticon commented Feb 4, 2022 •
edited
Loading

I fixed the exists() unit test by changing the query, since I do not have any influence over how and when the causing bug will be fixed in Polypheny DB.

As to handling of connections between Cottontail DB and Polypheny DB: I don't see an inconsistency here. JDBC connections are closed right away after an instance of a Wrapper is closed(). This is a bit different to gRPC, because the general recommendation for ManagedChannels is to keep them open for multiple calls. This is why there is some kind of connection pooling in the CottontailWrapper.

Edit: There was a (masked) problem with MetadataTest due to the wrapper being closed before each test, which means for Polypheny DB, that the JDBC connection is closed and subsequent tests fail (no effect for Cottontail DB).

@ppanopticon ppanopticon marked this pull request as ready for review February 4, 2022 11:26
@ppanopticon
Copy link
Member

I have addressed all the issues. This PR is ready for final review.

I will open an issue regarding DB connection handling in Cineast.

@ppanopticon ppanopticon mentioned this pull request Feb 4, 2022
Closed
lucaro
lucaro requested changes Feb 4, 2022
View reviewed changes
Copy link
Member

@lucaro lucaro left a comment

Choose a reason for hiding this comment

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

Looks all good, with the minor exception of the changes to the default config.

cineast.json Outdated Show resolved Hide resolved
lucaro
lucaro previously approved these changes Feb 4, 2022
View reviewed changes
@silvanheller
Copy link
Member Author

LGTM, thanks to everyone for all the work! I've tested this on the VBS data manually a bit and found no issues

@silvanheller silvanheller merged commit c450c76 into master Feb 7, 2022
@silvanheller silvanheller deleted the polyphenydb branch February 7, 2022 14:31
silvanheller added a commit that referenced this pull request Jun 10, 2022
@silvanheller @sauterl
New Cottontail API, PolyphenyDB (#224)
ac56496 
* Upgraded to new Cottontail API
* New Polypheny-DB Connector
* New DB Abstractions
* LSC Import Code which needed the new Cottontail API

Co-authored-by: Ralph Gasser <ralph.gasser@unibas.ch>
Co-authored-by: Loris Sauter <loris.sauter@unibas.ch>
Former-commit-id: c450c76
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lucaro lucaro lucaro approved these changes

@sauterl sauterl Awaiting requested review from sauterl

@ppanopticon ppanopticon Awaiting requested review from ppanopticon

Assignees

@ppanopticon ppanopticon

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@silvanheller @ppanopticon @sauterl @lucaro