Skip to content
This repository has been archived by the owner on Feb 16, 2021. It is now read-only.

[BETA] Migration to ActiveRecord-based Datastore #137

Merged
merged 25 commits into from Jun 26, 2018

Conversation

claudijd
Copy link
Contributor

Doing direct PG.connect calls really just isn't tenable after mucking with it for about a day or so. I'm coming to the conclusion that the API and it's DB/Store connectivity needs to be more abstracted and I feel like their has been entirely too much time spent in the weeds on this. Going to move over to using ActiveRecord as our DB interface, which supports all sorts of useful DB tech, like MySQL, PostgreSQL, SQLite, SQL Server, Sybase, and Oracle (all supported databases except DB2), should we decide to make a change our selves or if someone would prefer to use a different DB tech than us.

app.rb Outdated
s.target = params["target"]
s.port = 22
s.state = "QUEUED"
s.save
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is really so much cleaner and easier to read that the buried database abstraction layer.

@claudijd claudijd changed the title [WIP] Some initial work towards a full port to activerecord [WIP] Some initial work towards a full port to activerecord (one migration to rule them all) Jan 20, 2018
@claudijd claudijd changed the title [WIP] Some initial work towards a full port to activerecord (one migration to rule them all) [ALPHA] Some initial work towards a full port to activerecord (one migration to rule them all) Jan 30, 2018
@claudijd
Copy link
Contributor Author

ALPHA testers can follow instructions here:

https://github.com/mozilla/ssh_scan_api/wiki/Deploying-ssh_scan_api-using-docker-compose

@claudijd claudijd changed the title [ALPHA] Some initial work towards a full port to activerecord (one migration to rule them all) [ALPHA] Migration to ActiveRecord-based Datastore Jan 30, 2018
Copy link

@caggle caggle left a comment

Choose a reason for hiding this comment

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

Worked like a charm for me after following the instructions here: https://github.com/mozilla/ssh_scan_api/wiki/Deploying-ssh_scan_api-using-docker-compose

@claudijd
Copy link
Contributor Author

@caggle thanks for the quick review!

Copy link
Collaborator

@rishabhs95 rishabhs95 left a comment

Choose a reason for hiding this comment

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

This is amazing! Deploying the whole architecture was just so easy! Although I wanted to know your thoughts on the activerecord part. I agree that it provides a lot of flexibility on the database extension part but don't you think that it can add an extra overhead to the queries? Even I see this as a step forward(as we might not need that optimisation for now and it also saves a lot of effort for integrating different DBs) but just wanted to know your thoughts about this?

@claudijd
Copy link
Contributor Author

claudijd commented Feb 6, 2018

@rishabhs95 you are right, adding the whole of ActiveRecord introduces a lot more underly vendor code and perhaps gives us less "raw" performance on queries. That said, the way the infra is designed, I don't think we'll be bottle necking on any one area in particular, though I haven't perf tested any of this just yet. What I will say is that porting DB tech from mongodb to postgres was sort of a chore and were approaching (not yet hitting) performance walls in mongodb, which mainly driven by the /stats interface and the speed at which we could easily do data mining into the results.

One thing I do like, and you've hit on this point, is the ability to more easily switch to an alternative DB tech if we decide postgres isn't for us. I will say in some of the poking I have done, depending on how want to layout the database schema we can make it very postgres proprietary (like storing arrays and such) or we can keep it relatively generic allowing easy DB portability if someone really prefers mysql over postgres.

I suppose if ActiveRecord turns out to be a bottle neck here that would really suck, because this port is very "all in" with ActiveRecord and to get out, it would require another large set of changes like this. I'm really hoping it won't be (might require some testing to hedge our bet), but for the sake of simplicity in use, it's really slick. One of the things I noticed right off was the ability to temporarily lock records and roll back changes in the case we trip an exception mid-transaction and that is something we didn't have in the mongo setup, which resulted in a lot scans getting caught in running state.

As you might be able to tell by my comments, I'm still not 100% sure with this direction, there are some known pros and some possible cons we've yet to trip up on or measure. I'm actually a big advocate for experimentation on this type of stuff, so you want to play with it and see if you can measure the cons, I'm more than happy to participate and be learned up. My plan is to land this sometime in Q1, assuming it's all systems go.

@claudijd claudijd changed the title [ALPHA] Migration to ActiveRecord-based Datastore [BETA] Migration to ActiveRecord-based Datastore Jun 25, 2018
@claudijd claudijd merged commit 7dd52a0 into master Jun 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants