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

Initial draft at SQLite Backend #21

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

cddr
Copy link
Contributor

@cddr cddr commented Aug 15, 2016

No description provided.

(map-indexed (fn [i s]
(assoc s :sheet-name (str "Trial " (inc i))))
@outputs)})))
(jdbc/with-db-connection [db (sqlite/db-spec ":memory:")]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we move this down to the creation of the SQLiteBackend? I think that would make it easier to review by not changing the indent, and that's the only time we need the DB to be created.

Also, what are your thoughts on using memory vs. disk-backed DB. I feel like backing with disk is more interesting to trigger the IO behavior we might encounter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah memory was nice while getting started so I didn't have to worry about resetting the schema.

I didn't actually mean to submit a PR so early but thanks for your comments. I'll refactor the db lifecycle. This was just the easiest way to see some quick results. I'm not sure how to interpret that spreadsheet though.

(defn delete-key [db k]
(jdbc/delete! :hh-key ["k = ?" k]))

(defn add-refs [db {:keys [parent children]}]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like how you managed to get refcounting into SQLite, but I'm worried that the refcounting IO overhead might cause issues. On the other hand, maybe it won't. Have you ever profiled a JVM application? I'd be happy to teach you how to do so--the benchmarking tool makes it really easy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not something low-level like this. Would welcome the tutoring :-)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Great! Once you're able to run the benchmark, let's jump on a hangout and I'll walk you through some of the techniques I use to identify perf bottlenecks--we'll look for operations that use too much CPU, too much I/O, or cause weird cross-interferences :). You should have my google account from invite I requested on drive.

@cddr cddr changed the title Initial draft with benchmarking Initial draft at SQLite Backend Aug 15, 2016
@dgrnbrg
Copy link
Collaborator

dgrnbrg commented Sep 6, 2016

This is awesome. I think this is pretty much ready to merge, except for one thing: could you write a bit about the schema/structure of the data in SQLite, so that we can understand how the data is encoded?

@cddr
Copy link
Contributor Author

cddr commented Sep 7, 2016

Cool!

Sorry for the extra changes but I also took the liberty of renaming the namespace to be "hitchhiker.jdbc" because there's nothing specific to sqlite in there so it should just work with any database you can find a jdbc driver for.

There might be one other thing which to surface the selection of a backend to the user through outboard/create but I can do that in a new PR. I was thinking of adding a multi-method that dispatches off the name passed to outboard/create and returns a "backend constructor" and adding a slot in OutboardConnection to hold it.

@dgrnbrg
Copy link
Collaborator

dgrnbrg commented Sep 7, 2016

That looks great!

For Outboard, I had originally envisioned it as being a persistent in-memory backed data structure. Now that you bring it up, I can definitely see the value in extending the API to support plugging in alternate backends. Maybe the create function could take the backend as an argument, and then we could have namespaced functions like outboard.redis/create and outboard.jdbc/create for parametricity?

@cddr
Copy link
Contributor Author

cddr commented Sep 7, 2016

Maybe the create function could take the backend as an argument

That would also work but I thought create was supposed to be analogous to datomic's create-database (or would you want to avoid making the API exactly the same as datomic).

@dgrnbrg
Copy link
Collaborator

dgrnbrg commented Sep 7, 2016

Outboard != datomic, API-wise. Outboard's a proof-of-concept API to make the HH trees usable by end-users. There will be a different API that mirrors the Datomic API.

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