GitLab Database Office Hours, 24 Oct 2018

Previous Meeting Next Meeting

⏯

youtube image

►

From YouTube: Episode 2

Description

Episode 2 of Optimising Rails Database Queries. This time featuring a better microphone.

A

Hello and welcome back to episode, 2 of optimizing reals database queries in today's episode. We will take a brief look at the sequel query. We ended up with last episode, the sequel query. We want our code to produce and what changes we need to make to our code to make all of this happen. Now to the left. We have the C query that we ended up with. In the last episode and to the right. We have a sequel query that we want our code to produce there.

A

Quite a few differences, primarily creator write is much simpler and a lot of the redundant clauses or conditions etc that were applied in the old query have been removed. Some simple testing revealed that both produce the same data, the one to the left, takes about 15 seconds to run. We're still want to the right takes about 10 milliseconds to run now. The query to the right is basically Union. Together the results of three separate queries: I wrote each career separately and then just merge them together using a union.

A

The first one just gets snippets that we authored that don't belong to any project. Getting this is very simple, because we we don't need to deal with project access levels or any of that. The second case is, we want snippets of projects. We were one set member off. We could still see the project, even though we're no longer remember cases like that might be an open source project. You were a member of you left, but the project is public, and thus you should still be able to see the snippets you may have created.

A

The third case is to retrieve snippets of projects. You currently are a member of the fourth case that we don't show here is that if you're an administrator, you should just be able to see snippets of all projects if they have the snippets feature enabled the reason that's not here is because for that particular case, the query is so much simpler, that we basically use a different code path, and so we'll get to that when we start writing our code.

A

Now, we've written this sequel query, but we still need to adjust our code to actually produce it. So, let's start taking a look at how we do that now. In this case, the is located in a classical excuse me, a classical snippets finer now in get lap. We have these classical finders they're, essentially a sort of wrappers around active record models, and they take typically user or project and a variety of input, parameters from some kind of form or a URL and I based on those parameters.

A

They will build a database query that we use the actual data. Now we have the finer to the left here, and this particular finer is about a hundred fifty lines of code, excluding the various comments in it.

A

The wayfinders typically work is: they have a execute method, that's the one you call and that just returns some sort of active record relation. So if you look at this code, we start off using the init collection method, which has some logic where, if we give a project, we do it one way, and otherwise we do a different way and then we have to code to build those various steps. That is a bunch of access level checks involved.

A

So here we check if the current user has the wreath project snippet permission, and if so we check out- or you remember- then we do this, and otherwise we do that, etc.

A

There's quite a bit of logic, going on for reals five for any process of updating. There are some different optimizations for determining access lots of comments in general. If I, look at this code, I think that trying to modify this in place is gonna, make things very difficult. So what I'm gonna do instead is I'm. Just gonna comment out all of this code and I'm gonna start over. So the way we'll do that is. We will just take the sequel query. We have to the right and translate that to the closest Ruby code possible.

A

This is likely going to break a whole bunch of tests, but it's at least a starting point to see how far we managed to get with the current sequel query. So, first, we'll get the personals. That's I'm, just gonna write just like the sequel, three separate queries using the make the record over em and then we'll just Union those together. So your current use adult snippets to get these snippets of this person.

A

We want the ones that are not associated with projects where project is nil, equals to projectile is null. Then we have these snippets of X projects. We used to be member of that school X member snippets for a lack of a better name snippets. We want to join the projects.

A

I'm gonna write, Manuel, joins here, but later on, refactor all of these to reuse methods and use the proper join helpers and such for now I just want to get the closest possible code compared to the sequel. Query: we inner join project, features all project features. Project ID equals projects, ID I'm, not really gonna worry about line wrapping or any of that. For now. You want to limit this to the right. This ability level there has to be 10 and 20.

A

This we probably do have to wrap because it's going to be around a long line and then the disability levels. Let's look up what those were.

A

So we have private, is 0 internal as 10 and public is 20, so we need internal and public. So you get lab visibility, level, internal.

A

Public there we go there, we have to grab whoopsy. Then we have the snippets access level. So we do where project features snippets access level that has to be no 20 or 30, which corresponds to normal, enabled or public. The reason we have null there's just this column defaults to null, which we treat the same as the enabled which is fairly twenty video project, feature enabled and project feature public and the author ID filter is taken care by current user dot, snippets and I.

A

Finally, we have snippets of projects we are a member of so you, current user snippets. We can just copy over to joins, let's go down a little bit and then we have the snippet access level again, except this time. We also include private as a value.

A

We can do that here, because we are a member of the project. So if they're private, we still want to be able to see these snippets and then finally, we have this and exists Clause. So we do where exists.

A

Now. You can normally write this using Rossi cool I'm gonna use the ORM, because it's a little a little easier to deal with than having to break up a string across multiple lines, cuz. Otherwise it will get really long. So we do project authorization.

A

We won't select one we're using select one instead of select true because in the past, I had some issues where, if you use bully true, it's not always converted to the right type or reals, reject it. Something like that. So you wear project authorizations. The project ID equals the snippets project ID.

A

You could use something like an integer in here as well they're reasonably using this approach, because we have some methods for it. Is that take this particular approach, so it will be a little easier to refactor later on so snippets alternate ID there, yeah and I think then we have to Union those together. So for that we can do snippet from a union. That's a method. Real sorry get lot provides, so your personal snippets, ex-member snippets and the members snippets basically the way this method works.

A

It takes takes an array of active record relations and then just unions them together and then, finally, we do don't fresh, which is a method that applies the order by created at the sending.

A

Now we save that, and now we can take a look at how many of our tests we broke. By doing this I suspect quite a few will stop working, but hopefully maybe just one or two will be green. The left rubric up is complaining about a whole bunch of warnings. Allah will take care of this later on so a test finished. We had that's the three tests that seemed to have passed and basically all others failed. This isn't entirely surprising because we didn't take care awesome with the filtering.

A

That's done in the old approach, so, for example, there were some case where, oh, if you have certain permissions, you can do certain things or you cannot etc.

A

Let's take a look at the test to see what cases we we have to cover. So I am gonna. Open Snippets find your spec, so we are a tester right and we're just gonna work from top to bottom. So the first one is we filter by visibility. We say between public snippets when visibility is public.

A

So let's take a look at how we used to do that. Let's see there was a method for that called by visibility.

A

Yeah, since we're coming at it out that should be here somewhere, let's uncomment that so you can see so what I did is it took the visibility or some sort of default visibility, so we will copy that out as well, because we need that again and then it says: unless we give a visibility, we just return our input and otherwise we limit the input by the visibility level. Now that's two ways we can do this.

A

Since we use a union, we can apply to study individual snippets queries or we can apply to the outer query in this particular case. I think performance wise. It won't make that much of a difference so I'm just gonna apply to the outer world, but we later or may determined that we have to apply this to the individual queries. Instead, let's see that method was called. Was it again by visibility?

A

So we do life. Is ability items and then, if we go back to our terminal, we will just rerun the tests and see how many of them fail at this time and then, let's see so already, we fix the whole bunch of them. We're still dealing with quite a bunch of failures. Let's see so the first one that fails is on line 12. You can see that here snippers find out line 12.

A

So we go to that one and that is, it returns public snippets when visibility is public. Let's just run only.

A

That particular test, so we can see why that one is failing now generated ideas here. We we will keep using this sort of manually written code. We have two left and we'll try to make all the tests pass using this ugly code and then once they are all green, we can start refactoring it. The reason I'll do refactoring. This first is because, if I do that and test still fail, I won't be able to tell if they're failures because of the refactoring or the custom.

A

Query itself is broken now, in this case, it's saying undefined method snippets for nil class, which is probably because we do not pass a user. That's a little weird cuz here, current user is required. I know from experience. We typically pass nil, explicitly it's a it's a little weird, and so what I'm going to do is I'm. Just gonna make that optional. Just to make it clear that the value might be left out.

A

What that means is we have to do something like create a method called def snippets which doesn't like? If we have a user, we do we get the snippets and otherwise we just get, I suppose, all snippets. Let's see what the old coal used to do.

A

It's an inner collection. If we have a project we get the snippets for that project. Otherwise, we get the author of snippets and the author snippets method, let's just snippet we're public or visible to user.

A

Okay, so we can do and we don't really need a method just yet, although that's do it because we don't have to repeat stuff that way, so we do snip it public or visible to user current user.

A

Now, let's, let's double check this method actually does because I want to have as much control if the query as possible, so I want to make sure this doesn't start using like sub queries or stuff like that public or visible to user, and there we go so here we can see it already does some stuff. If authorizations and uses a sub query, that's all kind of stuff I really do not want because it makes it harder to optimize the screen.

A

However, the idea of all it does is something we have to replicate somehow, so what it does it says. So if we have a user, we want the snippets that the user is authorized to see what we really want in this case, because we take care of the filtering by authorization and visibility separately or just one is something like. If we have a user, we just get that user snippet.

A

Otherwise we get all the snippets that are public to user method is called little. Weird in this case call so no passing use there, but in this case that means I believe it'll just get public snippets, that's it but I'm trying to see where that method is defined public to new user.

A

That does not appear to be defined anywhere, so that is probably a some sort of helper method from a module public to user. Let's see what it's defined public to user see snippets are there we go it's a scope in the visibility lawful. That just says where the visibility level is in a particular list for a user.

A

So we can go back to our terminal, see where they go.

A

So we closed that, so we have here a mythical Def snippets. We just do if current user, Kurt user, snippets I suppose we could rewrite that to current use, adult snippets or snippet public to user fusion. We can do this. The I forgot what the operators call I, think the the lazy operator or something basically, if you do ampersand dot. If current user is nil, this expression will just return now, but if it's not, they will send snippets to good user, and then we can just write a one-line or statement.

A

We don't need to change all case of current user snippets to just snippets, so we do that there we go, then we have one more here and we can run our test again. Let's see how many are failing this time and we will just repeat this process over and over and over until we have all green tests and then we can start refactoring everything, so the next one that fails is 61. Let's take a look at that one.

A

The reason I'm taking these tests individually is it's a little easier than to look at the entire test. Filler, your output and, let's see where this one is failing, so this one expects a personal snippet with ID 2 & 3, but it doesn't get anything no test. Does it says, filtered by author returns all public in internal snippets?

A

Let's take a look at that actual test. Let's see what it's doing so we're filtering back welfare. Let's see we create snippets and then we say: oh well, no so very specific, author, not the current one.

A

That's there's a little weird case because I think we don't actually use that anywhere and it's especially weird since we have like we default buddy user, but if we supply an all through okay. So what we then do is who do something like eighth pair M's, altar and I? Suppose that's, probably a user object of sorts. Let's see, we had a method for that.

A

So what this used to do is it set by other items for doing items on the spams altar? Oh yeah, that's so that is an author object? Okay, so we can do.

A

This is a little bit if we simply apply that method we have below here the by author method. What happens is our snippets method will apply something like a we're all three IDs X and then later on? We do something like and all that IDE is y, which is a little redundant because we can just wear all three DS Y. So instead of using this by author method, I'm gonna apply it here in snippets.

A

I'm gonna do something like if parents author and we do parents, author snippets else, do the rest and this in turn we can change to parents or thir snippets again with the lazy operator or the current user or the snippets that are just public to everybody, and then we can wrap this okay.

A

So again, let's try that and we'll start with just running the test at 161 and we'll see if that gets us anywhere and that one is still failing, and this time we have a few too many snippets. Apparently that's interesting, let's see so active record a appears to be including snippet twice. That's interesting.

A

Let's see so expected blah blah blah to not include alright okay, so we return one snippet too many see which one that is so we go into line 61. So snippet one is private. Oh.

A

Okay, I get it so.

A

The particular issue here is that the occurred user is used for the authorization checks so for private projects. We're going to make sure that the current user can view those, but they are not necessarily also the the author of those snippets. So probably what we need to do is.

A

Change this where' exists. Let's see so, instead of this being scope to the snippets author ID, there should be scoped where project authorizations user ID equals the ID of the current user.

A

Current user, the ID- and this will not probably fail if, if you are not logged in and that probably means that some other test will fail, but this one might now pass.

A

So we from there again- and it still fails now in self spending. You know the next 45 minutes trying to get these tests to work.

A

I'll just sort of briefly highlight the idea, because I I don't think a video of two hours of fixing test is gonna, be terribly interesting, but generally the idea we're trying to apply here. So we try to translate our a see Coco to Ruby code as close as possible, and then you sort of gradually try to refactor this, so it meets all the requirements now the reason I'm not using any of this old code. It's just that particular code use all kinds of sub queries and all kinds of conditions that we might not need.

A

If you just slightly rewrite our query so ya study, we write our code and so I find that a certain case, so starting from scratch, is just a little bit easier to wrap your head around than trying to sort of squeeze optimizations into an existing code base.

A

This does mean that sometimes it's gonna be quite a bit more work. So, based on these test failures and the code in front of me, I would expect that trying to make this work is probably going to take like an hour or two. However, that would be one or two hours that you have to do only once and then hopefully, after that, you never have to touch this code again. Compare it with the old approach where we use various sub queries.

A

And/Or statements I find that performance wise those are very sensitive to changes, so you might change one thing in one area and unexpectedly slow things down elsewhere, whereas with this approach of taking here it's much more contained because we we specifically write this finer for this very specific use case. Instead of trying to reuse as many different things possible, there doesn't mean code reuse as bad.

A

It has great value, but there has to be balanced between reuse and specific cope with a specific purpose for specific use case, it's basically impossible to write one method that does everything in an efficient way. It's inevitable that somebody will come along and say: oh hey! This method should now also do this extra case, and necessarily it might slow down everything. So you have to sort of balance between how much you want to reuse and how much you want to write for a specific purpose.

A

My experience typically in rails projects, people tend to lean towards reusing more than they should so in gillip. We have some rules set up against this where we say. Oh, certain methods can be reused in certain contacts and we have Robocop warning else for that. So all these little warnings here we see to the left, let's rubric up telling us that, for example, surgeon methods can only be used inside an actor's record model, the idea so sort of push logic in certain places so that it's easier to refactor.

A

For example, if you rename a column is easier to do that, cuz fewer places where that column might be mentioned directly now, if not all, set I'm gonna close, this episode, I will fix this code for the next episode and then we'll take a look there at what we ended up with. Why and sort of briefly do a code review of sorts to sort of explain the thought process behind that. So thank you for watching and we'll see you in the next episode.