Add workaround for MySQL bug

[SuperQ]

Workaround bug reported by #127.

• Use a sub-select and group by to limit to one digest/schema pair.

[SuperQ]

@beorn7 This fixes the bug, but we don't have a test file for this collector yet. I will add one.

[beorn7]

Q? Is that the super-Q?

[SuperQ]

Hah, no, it's an implementation detail of having to assign the sub-query to a virtual table.

[beorn7]

My SQL foo is weak.

I just have to believe you that the query is correct.

[beorn7]

We should also use a custom handler in https://github.com/prometheus/mysqld_exporter/blob/master/mysqld_exporter.go#L446 . One that continues on error but logs the errors so that we get metrics even if MySQL misbehaves, but we also see them in the log to be aware of them.

[brian-brazil]

mysqld_up should be 0 in that case.

[beorn7]

If a collection fails outright, we can do that. But if the collection results in inconsistent metrics (which is only detected once the collection is over), we cannot insert new metrics at that point (or let's say that would require a new feature in the client library).

[brian-brazil]

The way I generally handle that is only to send the _up at the very end of the collector, when we know if things have worked.

[beorn7]

Yes, that works within a collector. But if inconsistent metrics are created for some reason the collector hasn't detected, it's only detected once all the metrics are ready for shipping. Inserting an additional metric then is an interesting approach to communicate that problem but that's not implemented yet.
By default, we return 500 and no metrics. But right now, you can configure to return what's there and log the error.

[beorn7]

We can go with the change in this PR for now, but keep in mind that the scrape will result in 500 in case of any inconsistencies (which is arguably better than the previous state).

[SuperQ]

With the new query, the previous issues will be gone.

[beorn7]

👍 Let's discuss error handling separately later.

[jeremy]

Still seeing duplicates with this query. Two options:

• Use rank/partition to limit the subquery to a single row per digest. A bit fiddly. Easier on MySQL 8 with row_number().
• Assume multiple digests rather than trying to eliminate them. Use aggregate SUM() to combine summary data. This is simpler and doesn't need a subquery.

Example with row_number (MySQL 8 only) to select just the latest row per digest:

@@ -16,28 +16,14 @@
 	    SUM_SORT_ROWS,
 	    SUM_NO_INDEX_USED
 	  FROM (
-	    SELECT *
+	    SELECT *, row_number() over (partition by DIGEST order by last_seen DESC)
+
 	    FROM performance_schema.events_statements_summary_by_digest
 	    WHERE SCHEMA_NAME NOT IN ('mysql', 'performance_schema', 'information_schema')
 	      AND LAST_SEEN > DATE_SUB(NOW(), INTERVAL %d SECOND)
-	    ORDER BY LAST_SEEN DESC
+	    GROUP BY SCHEMA_NAME, DIGEST
 	  )Q
-	  GROUP BY
-	    Q.SCHEMA_NAME,
-	    Q.DIGEST,
-	    Q.DIGEST_TEXT,
-	    Q.COUNT_STAR,
-	    Q.SUM_TIMER_WAIT,
-	    Q.SUM_ERRORS,
-	    Q.SUM_WARNINGS,
-	    Q.SUM_ROWS_AFFECTED,
-	    Q.SUM_ROWS_SENT,
-	    Q.SUM_ROWS_EXAMINED,
-	    Q.SUM_CREATED_TMP_DISK_TABLES,
-	    Q.SUM_CREATED_TMP_TABLES,
-	    Q.SUM_SORT_MERGE_PASSES,
-	    Q.SUM_SORT_ROWS,
-	    Q.SUM_NO_INDEX_USED
+	  WHERE row_number = 1
 	  ORDER BY SUM_TIMER_WAIT DESC
 	  LIMIT %d
 	`

Example with no subquery and simply SUM() the rows:

@@ -3,41 +3,22 @@
 	    ifnull(SCHEMA_NAME, 'NONE') as SCHEMA_NAME,
 	    DIGEST,
 	    LEFT(DIGEST_TEXT, %d) as DIGEST_TEXT,
-	    COUNT_STAR,
-	    SUM_TIMER_WAIT,
-	    SUM_ERRORS,
-	    SUM_WARNINGS,
-	    SUM_ROWS_AFFECTED,
-	    SUM_ROWS_SENT,
-	    SUM_ROWS_EXAMINED,
-	    SUM_CREATED_TMP_DISK_TABLES,
-	    SUM_CREATED_TMP_TABLES,
-	    SUM_SORT_MERGE_PASSES,
-	    SUM_SORT_ROWS,
-	    SUM_NO_INDEX_USED
-	  FROM (
-	    SELECT *
-	    FROM performance_schema.events_statements_summary_by_digest
-	    WHERE SCHEMA_NAME NOT IN ('mysql', 'performance_schema', 'information_schema')
-	      AND LAST_SEEN > DATE_SUB(NOW(), INTERVAL %d SECOND)
-	    ORDER BY LAST_SEEN DESC
-	  )Q
-	  GROUP BY
-	    Q.SCHEMA_NAME,
-	    Q.DIGEST,
-	    Q.DIGEST_TEXT,
-	    Q.COUNT_STAR,
-	    Q.SUM_TIMER_WAIT,
-	    Q.SUM_ERRORS,
-	    Q.SUM_WARNINGS,
-	    Q.SUM_ROWS_AFFECTED,
-	    Q.SUM_ROWS_SENT,
-	    Q.SUM_ROWS_EXAMINED,
-	    Q.SUM_CREATED_TMP_DISK_TABLES,
-	    Q.SUM_CREATED_TMP_TABLES,
-	    Q.SUM_SORT_MERGE_PASSES,
-	    Q.SUM_SORT_ROWS,
-	    Q.SUM_NO_INDEX_USED
+	    SUM(COUNT_STAR) COUNT_STAR,
+	    SUM(SUM_TIMER_WAIT) SUM_TIMER_WAIT,
+	    SUM(SUM_ERRORS) SUM_ERRORS,
+	    SUM(SUM_WARNINGS) SUM_WARNINGS,
+	    SUM(SUM_ROWS_AFFECTED) SUM_ROWS_AFFECTED,
+	    SUM(SUM_ROWS_SENT) SUM_ROWS_SENT,
+	    SUM(SUM_ROWS_EXAMINED) SUM_ROWS_EXAMINED,
+	    SUM(SUM_CREATED_TMP_DISK_TABLES) SUM_CREATED_TMP_DISK_TABLES,
+	    SUM(SUM_CREATED_TMP_TABLES) SUM_CREATED_TMP_TABLES,
+	    SUM(SUM_SORT_MERGE_PASSES) SUM_SORT_MERGE_PASSES,
+	    SUM(SUM_SORT_ROWS) SUM_SORT_ROWS,
+	    SUM(SUM_NO_INDEX_USED) SUM_NO_INDEX_USED
+	  FROM performance_schema.events_statements_summary_by_digest
+	  WHERE SCHEMA_NAME NOT IN ('mysql', 'performance_schema', 'information_schema')
+	    AND LAST_SEEN > DATE_SUB(NOW(), INTERVAL %d SECOND)
+	  GROUP BY SCHEMA_NAME, DIGEST, DIGEST_TEXT
 	  ORDER BY SUM_TIMER_WAIT DESC
 	  LIMIT %d
 	`