-
Notifications
You must be signed in to change notification settings - Fork 333
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
feat(audit-logs/search): Add filter using log and environments #633
Conversation
Yes I think if we can manage to get away with out a text index so that we don't break compatibility with other DBMS's we should try and work to that goal. I'm assuming PG will run the filter on Env ID before doing the text search - which will cut down the query time a lot, especially on SaaS. Are we able to run some tests with e.g. 10,000 log record in an environment? |
I did not try that, since the query planer takes a lot of things into considerations before generating a query plan, which kind of makes my local benchmarks a bit unreal liable? That's why I wanted to run the SQL on production |
it would be good to run an |
I created a gin index, but it seems like postgres find other index much faster. Below are some query plans generated by the same query on different dataset:
It is doing a sequential scan on audit log because the dataset is small, and it does not make much sense to use any index here. 2.) 2million rows:
Now the query plan is wildly different from the one generated for 1k rows, and as you can see it is using the project_id index on audit log. So to sum it all up, adding a gin index at this point has more downsides(because of multiple databases and also because it's not being used) than upside and the performance is not bad for a dataset of 2M rows. |
We should go ahead with this, but have the FE behind a feature flag (so deciding whether to do the search client side as currently does, or server side as per this new endpoint). Then we can test performance in prod. One further option would be to default limit the audit log search to last 30 days which I think would be acceptable. |
api/audit/tests/test_views.py
Outdated
assert response.json()["results"][1]["environment"] is None | ||
|
||
|
||
def test_audit_log_can_be_filtered_by_related_object_type( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't really filtering by 'related object type' right?
1c5e446
to
eb97eba
Compare
* feat(audit-logs/search): Add filter using log and environment * Add tests * fix/improve tests
Accepts
search
as query parms to perform search onlog
.Since the audit log table is huge, I think we should run the SQL on prod database to see the performance once before rolling it out to production.
Generated sql looks like this:
Full text search approach:
I think full text search approach seems kinda overkill right now because of multiple reasons:
Ref: #408