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

Call ANALYZE less often #3518

Closed
Tracked by #75
rumyantseva opened this issue Oct 6, 2023 · 7 comments · Fixed by #3563
Closed
Tracked by #75

Call ANALYZE less often #3518

rumyantseva opened this issue Oct 6, 2023 · 7 comments · Fixed by #3563
Assignees
Labels
area/diag Issues about diagnostic commands code/chore Code maintenance improvements community Issues and PRs assigned to community members good first issue Good issues for new external contributors
Milestone

Comments

@rumyantseva
Copy link
Member

rumyantseva commented Oct 6, 2023

What should be done?

PostgreSQL and SQLite backends call ANALYZE on the whole database when dbStats and collStats commands are called. That's slow and not necessary in real usage.

Let's call ANALYZE only when Database.Stats and Collection.Stats are called with Refresh: true. dbStats and collStats should call those methods with Refresh: true for now (to do less in this issue), but we might change that in the future.

Additionally, we should call ANALYZE only on a given list of collections (and given PostgreSQL schema), not on the whole database.

Where?

See references to this issue.

Definition of Done

  • new SQLite handler updated;
  • new PostgreSQL and SQLite backends updated;
  • spot refactorings done.
@rumyantseva rumyantseva added code/chore Code maintenance improvements not ready Issues that are not ready to be worked on; PRs that should skip CI labels Oct 6, 2023
@rumyantseva
Copy link
Member Author

From logs:

...
ferretdb_postgres          | 2023-10-06 18:05:12.887 NDT [1929] LOG:  duration: 1022.363 ms  statement: ANALYZE
ferretdb_postgres          | 2023-10-06 18:05:12.959 NDT [1933] LOG:  duration: 1022.894 ms  statement: ANALYZE
ferretdb_postgres          | 2023-10-06 18:05:13.131 NDT [1937] LOG:  duration: 1028.898 ms  statement: ANALYZE
ferretdb_postgres          | 2023-10-06 18:05:13.229 NDT [1909] LOG:  duration: 1023.233 ms  statement: ANALYZE
ferretdb_postgres          | 2023-10-06 18:05:13.383 NDT [1913] LOG:  duration: 1030.940 ms  statement: ANALYZE
ferretdb_postgres          | 2023-10-06 18:05:13.519 NDT [1917] LOG:  duration: 1026.530 ms  statement: ANALYZE
ferretdb_postgres          | 2023-10-06 18:05:13.603 NDT [1921] LOG:  duration: 1019.468 ms  statement: ANALYZE
ferretdb_postgres          | 2023-10-06 18:05:13.772 NDT [1925] LOG:  duration: 1027.525 ms  statement: ANALYZE
ferretdb_postgres          | 2023-10-06 18:05:13.915 NDT [1929] LOG:  duration: 1021.194 ms  statement: ANALYZE
ferretdb_postgres          | 2023-10-06 18:05:13.989 NDT [1933] LOG:  duration: 1024.506 ms  statement: ANALYZE
ferretdb_postgres          | 2023-10-06 18:05:14.155 NDT [1937] LOG:  duration: 1019.263 ms  statement: ANALYZE
ferretdb_postgres          | 2023-10-06 18:05:14.260 NDT [1909] LOG:  duration: 1026.028 ms  statement: ANALYZE
ferretdb_postgres          | 2023-10-06 18:05:14.411 NDT [1913] LOG:  duration: 1022.705 ms  statement: ANALYZE
ferretdb_postgres          | 2023-10-06 18:05:14.548 NDT [1917] LOG:  duration: 1023.588 ms  statement: ANALYZE
ferretdb_postgres          | 2023-10-06 18:05:14.631 NDT [1921] LOG:  duration: 1022.006 ms  statement: ANALYZE
ferretdb_postgres          | 2023-10-06 18:05:14.810 NDT [1925] LOG:  duration: 1032.605 ms  statement: ANALYZE
ferretdb_postgres          | 2023-10-06 18:05:14.933 NDT [1929] LOG:  duration: 1012.820 ms  statement: ANALYZE
ferretdb_postgres          | 2023-10-06 18:05:15.010 NDT [1933] LOG:  duration: 1015.881 ms  statement: ANALYZE
...

But I'm not sure how to reproduce without running many tests in parallel.

@AlekSi AlekSi changed the title Don't call ListDatabases from tests when checking connection Call ANALYZE less Oct 7, 2023
@AlekSi AlekSi added good first issue Good issues for new external contributors hacktoberfest and removed not ready Issues that are not ready to be worked on; PRs that should skip CI labels Oct 7, 2023
@Aditya1404Sal
Copy link
Contributor

Hey @rumyantseva i'm interested in working on this issue.

@rumyantseva
Copy link
Member Author

@Aditya1404Sal thank you for your interest! I assigned you, so please feel free to take it.

@AlekSi AlekSi added the community Issues and PRs assigned to community members label Oct 9, 2023
@AlekSi AlekSi added the area/diag Issues about diagnostic commands label Oct 10, 2023
@AlekSi AlekSi changed the title Call ANALYZE less Call ANALYZE less often Oct 13, 2023
@Aditya1404Sal
Copy link
Contributor

To set the Refresh : true for dbstat and simillarly for collStat
is a constructor like this okay ?

func NewDatabaseStatsParams() DatabaseStatsParams {
    return DatabaseStatsParams{
        Refresh: true,
    }
}

@AlekSi
Copy link
Member

AlekSi commented Oct 16, 2023

I don't think we need any constructor: both the type and the field are exported and directly accessible, and the zero value is good enough.

@Aditya1404Sal
Copy link
Contributor

For the postgresql backend , ANALYZE schema.tablename1 ........ should be the correct syntax , What is the schema declared as ?

@AlekSi
Copy link
Member

AlekSi commented Oct 17, 2023

PostgreSQL schema name is the same as FerretDB database name

AlekSi added a commit that referenced this issue Oct 19, 2023
Closes #3518.

Co-authored-by: Alexey Palazhchenko <alexey.palazhchenko@ferretdb.io>
Co-authored-by: Chi Fujii <chi.fujii@ferretdb.io>
@AlekSi AlekSi added this to the v1.13.0 milestone Oct 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/diag Issues about diagnostic commands code/chore Code maintenance improvements community Issues and PRs assigned to community members good first issue Good issues for new external contributors
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants