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

Jump to bottom

SQLStore: Support Upserting multiple rows. #52228

Merged
merged 4 commits into from Jul 19, 2022
Merged

SQLStore: Support Upserting multiple rows. #52228

merged 4 commits into from Jul 19, 2022

Conversation

joeblubaugh
Copy link
Contributor

This will be used to reduce write load when the alerting system writes a
large number of events, like alert instances.

Prompted by https://twitter.com/ded787/status/1542820281470820354

A follow-up PR will use this code in the alerting system.

@joeblubaugh
SQLStore: Support Upserting multiple rows.
02fb2b9 
This will be used to reduce write load when the alerting system writes a
large number of events.
@joeblubaugh joeblubaugh requested a review from a team as a code owner July 14, 2022 12:20
@joeblubaugh joeblubaugh requested review from kylebrandt, zserge and mildwonkey and removed request for a team July 14, 2022 12:20
@joeblubaugh joeblubaugh added this to the 9.1.0 milestone Jul 14, 2022
@grafanabot
Copy link
Contributor

Drone build failed: https://drone.grafana.net/grafana/grafana-enterprise/26208

@yuri-tceretian
Copy link
Contributor

LGTM. I would like backend team to take a look, especially @papagian and\or @kylebrandt

papagian
papagian approved these changes Jul 19, 2022
View reviewed changes
Copy link
Contributor

@papagian papagian left a comment

Choose a reason for hiding this comment

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

LGTM

pkg/services/sqlstore/migrator/mysql_dialect.go
@@ -226,13 +234,23 @@ func (db *MySQLDialect) UpsertSQL(tableName string, keyCols, updateCols []string
setStr.WriteString(fmt.Sprintf("%s=VALUES(%s)%s", db.Quote(c), db.Quote(c), separator))
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that starting from MySQL 8.0.20 the use of VALUES() to refer to the new row and columns is deprecated and is subject from removal in the future.
So ideally, we have to use row and column aliases (introduced in MySQL 8.0.19) for supporting the latest MySQL versions and fallback to VALUES() when using older versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The new syntax is this close to the Postgres syntax, but not quite. That's too bad.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've filed #52437

pkg/services/sqlstore/migrator/mysql_dialect.go Outdated Show resolved Hide resolved
@papagian
Copy link
Contributor

given it targets milestone 9.1.0 we need to change the backport label to no-backport.

@joeblubaugh joeblubaugh mentioned this pull request Jul 19, 2022
Closed
@joeblubaugh @papagian
Update pkg/services/sqlstore/migrator/mysql_dialect.go
1e487da 
Co-authored-by: Sofia Papagiannaki <1632407+papagian@users.noreply.github.com>
@grafanabot
Copy link
Contributor

Drone build failed: https://drone.grafana.net/grafana/grafana-enterprise/26714

@joeblubaugh joeblubaugh modified the milestones: 9.1.0, 9.0.4 Jul 19, 2022
@joeblubaugh joeblubaugh merged commit a0f96ed into main Jul 19, 2022
@joeblubaugh joeblubaugh deleted the joe/sqlstore-multiple-upsert branch July 19, 2022 08:42
@grafanabot grafanabot mentioned this pull request Jul 19, 2022
Merged
grafanabot pushed a commit that referenced this pull request Jul 19, 2022
@joeblubaugh @grafanabot
SQLStore: Support Upserting multiple rows. (#52228)
f1e9f42 
This will be used to reduce write load when the alerting system writes a
large number of events.

(cherry picked from commit a0f96ed)
joeblubaugh added a commit that referenced this pull request Jul 19, 2022
@grafanabot @joeblubaugh
SQLStore: Support Upserting multiple rows. (#52228) (#52438)
b026345 
This will be used to reduce write load when the alerting system writes a
large number of events.

(cherry picked from commit a0f96ed)

Co-authored-by: Joe Blubaugh <joe.blubaugh@grafana.com>
avodotiiets pushed a commit to logzio/data-viz that referenced this pull request Nov 4, 2022
@joeblubaugh
SQLStore: Support Upserting multiple rows. (grafana#52228)
f2f8e54 
This will be used to reduce write load when the alerting system writes a
large number of events.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chaudyg chaudyg Awaiting requested review from chaudyg

@yuri-tceretian yuri-tceretian Awaiting requested review from yuri-tceretian

@kylebrandt kylebrandt Awaiting requested review from kylebrandt kylebrandt was automatically assigned from grafana/backend-platform

@zserge zserge Awaiting requested review from zserge zserge was automatically assigned from grafana/backend-platform

@mildwonkey mildwonkey Awaiting requested review from mildwonkey mildwonkey was automatically assigned from grafana/backend-platform

@papagian papagian Awaiting requested review from papagian

Assignees
No one assigned
Labels
add to changelog area/alerting Grafana Alerting area/backend backport v9.0.x enterprise-ok kind/enhancement
Projects
Alerting
Archived in project
Milestone
9.0.4
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@joeblubaugh @grafanabot @yuri-tceretian @papagian