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

[TT-9468] New SQL Aggregate indexes #694

Merged
merged 23 commits into from
Aug 29, 2023
Merged

Conversation

tbuchaillot
Copy link
Contributor

@tbuchaillot tbuchaillot commented Jul 27, 2023

Description

This PR drops the dimension index for sql_aggregate pumps and creates a new one called idx_dimension. This change is made so the index we are using has the same order as Tyk Dashboard queries.

In order to avoid blocking production table, we execute the index creation in a goroutine when the database is postgres and use the special postgres instruction CONCURRENTLY to avoid table locking. More info about this here

Related Issue

https://tyktech.atlassian.net/browse/TT-9468

Motivation and Context

https://tyktech.atlassian.net/browse/TT-9468

How This Has Been Tested

Added unit tests for SQLite scenarios (we don't have Postgres running on CI)
Tested manually

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • Make sure you are requesting to pull a topic/feature/bugfix branch (right side). If pulling from your own
    fork, don't request your master!
  • Make sure you are making a pull request against the master branch (left side). Also, you should start
    your branch off our latest master.
  • My change requires a change to the documentation.
    • If you've changed APIs, describe what needs to be updated in the documentation.
  • I have updated the documentation accordingly.
  • Modules and vendor dependencies have been updated; run go mod tidy && go mod vendor
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • Check your code additions will not fail linting checks:
    • go fmt -s
    • go vet

@tbuchaillot tbuchaillot requested a review from a team as a code owner July 27, 2023 12:48
@alephnull alephnull removed the request for review from a team August 1, 2023 06:03
@tbuchaillot tbuchaillot changed the title fixing SQL Aggregate indexes ]New SQL Aggregate indexes Aug 1, 2023
@tbuchaillot tbuchaillot changed the title ]New SQL Aggregate indexes [TT-9468] New SQL Aggregate indexes Aug 1, 2023
.github/workflows/linter.yaml Outdated Show resolved Hide resolved
pumps/sql_aggregate.go Show resolved Hide resolved
pumps/sql_aggregate_test.go Show resolved Hide resolved
Copy link
Contributor

@sredxny sredxny left a comment

Choose a reason for hiding this comment

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

lgtm

@tbuchaillot tbuchaillot merged commit 657436f into master Aug 29, 2023
10 checks passed
@tbuchaillot tbuchaillot deleted the fix/sqlaggregate-index branch August 29, 2023 08:21
@tbuchaillot
Copy link
Contributor Author

/release to release-1.8

@tykbot
Copy link

tykbot bot commented Aug 29, 2023

Working on it! Note that it can take a few minutes.

tykbot bot pushed a commit that referenced this pull request Aug 29, 2023
* fixing SQL Aggregate indexes

* TT-9490 change name from APISchema to ApiSchema (#681)

* change name from APISchema to ApiSchema

* update regex to blur mongo credentials (#686)

* updated prometheus to 1.16 (#688)

* Auto generated from templates by gromit (#692)

Co-authored-by: Gromit <policy@gromit>

* [TT-9464] Updating storage to v1.0.7 (#689)

* Updating storage to v1.0.7

* go mod tidy

* fixing SQL Aggregate indexes

* removing unused func

* linting

* adding waiting time for sqlite

* using background only on postgres

* adding comment

* updating comments

* restoring linter.yaml

* adding index created on non existing table, not background scenario

* adding dbType var from config

* taking care of side scenario

* controlling bg chan

* adding omit_index_creation

---------

Co-authored-by: Sredny M <sredny.buitrago@gmail.com>
Co-authored-by: Alok G Singh <alephnull@hcoop.net>
Co-authored-by: Gromit <policy@gromit>
Co-authored-by: Matias <83959431+mativm02@users.noreply.github.com>

(cherry picked from commit 657436f)
@tykbot
Copy link

tykbot bot commented Aug 29, 2023

@tbuchaillot Succesfully merged PR

buger added a commit that referenced this pull request Aug 29, 2023
[TT-9468] New SQL Aggregate indexes (#694)

* fixing SQL Aggregate indexes

* TT-9490 change name from APISchema to ApiSchema (#681)

* change name from APISchema to ApiSchema

* update regex to blur mongo credentials (#686)

* updated prometheus to 1.16 (#688)

* Auto generated from templates by gromit (#692)

Co-authored-by: Gromit <policy@gromit>

* [TT-9464] Updating storage to v1.0.7 (#689)

* Updating storage to v1.0.7

* go mod tidy

* fixing SQL Aggregate indexes

* removing unused func

* linting

* adding waiting time for sqlite

* using background only on postgres

* adding comment

* updating comments

* restoring linter.yaml

* adding index created on non existing table, not background scenario

* adding dbType var from config

* taking care of side scenario

* controlling bg chan

* adding omit_index_creation

---------

Co-authored-by: Sredny M <sredny.buitrago@gmail.com>
Co-authored-by: Alok G Singh <alephnull@hcoop.net>
Co-authored-by: Gromit <policy@gromit>
Co-authored-by: Matias <83959431+mativm02@users.noreply.github.com>
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.

4 participants