Skip to content
This repository has been archived by the owner on Apr 2, 2024. It is now read-only.

Commit

Permalink
Prevent concurrent table creation errors for long metric name
Browse files Browse the repository at this point in the history
Previously, when two metrics with really long names were created
concurrently, postgres could throw errors due to name collision
violations when it created constraint names as well as type names
for the type corresponding to the table.

Constraint names was fixed by explicitly assigning a unique constraint
name instead of having PG autogenerate one.

Types are a bit trickier. Turns out for every table X, PG assigns
an array type '_X' (makeArrayTypeName() in PG source). That can
also collide and cause erroors. The fix was for us to change the
max length of the table names we geneate from 63 chars to 62 chars,
thus reserving one char for the underscore.
  • Loading branch information
cevian committed Jun 4, 2020
1 parent 481e7cd commit 4843f8c
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 12 deletions.
36 changes: 33 additions & 3 deletions pkg/pgmodel/end_to_end_tests/concurrent_sql_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package end_to_end_tests
import (
"context"
"fmt"
"strings"
"sync"
"testing"

Expand Down Expand Up @@ -135,21 +136,50 @@ func TestConcurrentSQL(t *testing.T) {
t.Fatalf("ids aren't equal: %d != %d", id1, id2)
}

wg.Add(2)
/* test creating tables with same long name */
wg.Add(3)
go func() {
defer wg.Done()
_, err := db.Exec(context.Background(), "CALL _prom_catalog.finalize_metric_creation()")
if err != nil {
t.Fatal(err)
}
}()
metric := fmt.Sprintf("test1"+strings.Repeat("long_name_", 10)+"goc_metric_%d", i)
go func() {
defer wg.Done()
id1 = testConcurrentGOCMetricTable(t, db, fmt.Sprintf("goc_metric_%d", i))
id1 = testConcurrentGOCMetricTable(t, db, metric)
}()
go func() {
defer wg.Done()
id2 = testConcurrentGOCMetricTable(t, db, fmt.Sprintf("goc_metric_%d", i))
id2 = testConcurrentGOCMetricTable(t, db, metric)
}()
wg.Wait()

if id1 != id2 {
t.Fatalf("ids aren't equal: %d != %d", id1, id2)
}

/* test creating tables with long names and different suffixes */
wg.Add(3)
go func() {
defer wg.Done()
_, err := db.Exec(context.Background(), "CALL _prom_catalog.finalize_metric_creation()")
if err != nil {
t.Fatal(err)
}
}()
metric = fmt.Sprintf("test2"+strings.Repeat("long_name_", 10)+"goc_metric_%d", i)
go func() {
defer wg.Done()
id1 = testConcurrentGOCMetricTable(t, db, metric)
}()
go func() {
defer wg.Done()
id2 = testConcurrentGOCMetricTable(t, db, metric+"_diff")
}()
wg.Wait()

wg.Add(2)
go func() {
defer wg.Done()
Expand Down
4 changes: 2 additions & 2 deletions pkg/pgmodel/migrations/migration_files_generated.go

Large diffs are not rendered by default.

18 changes: 11 additions & 7 deletions pkg/pgmodel/migrations/sql/1_base_schema.up.sql
Original file line number Diff line number Diff line change
Expand Up @@ -282,8 +282,8 @@ BEGIN
labels SCHEMA_PROM.label_array NOT NULL,
CHECK(labels[1] = %2$L AND labels[1] IS NOT NULL),
CHECK(metric_id = %3$L),
UNIQUE(labels) INCLUDE (id),
PRIMARY KEY(id)
CONSTRAINT series_labels_id_%3$s UNIQUE(labels) INCLUDE (id),
CONSTRAINT series_pkey_%3$s PRIMARY KEY(id)
)
$$, NEW.table_name, label_id, NEW.id);

Expand All @@ -307,12 +307,12 @@ CREATE TRIGGER make_metric_table_trigger

-- Return a table name built from a full_name and a suffix.
-- The full name is truncated so that the suffix could fit in full.
-- name size will always be exactly 63 chars.
-- name size will always be exactly 62 chars.
CREATE OR REPLACE FUNCTION SCHEMA_CATALOG.pg_name_with_suffix(
full_name text, suffix text)
RETURNS name
AS $func$
SELECT (substring(full_name for 63-(char_length(suffix)+1)) || '_' || suffix)::name
SELECT (substring(full_name for 62-(char_length(suffix)+1)) || '_' || suffix)::name
$func$
LANGUAGE SQL IMMUTABLE PARALLEL SAFE;
GRANT EXECUTE ON FUNCTION SCHEMA_CATALOG.pg_name_with_suffix(text, text) TO prom_reader;
Expand All @@ -322,14 +322,18 @@ GRANT EXECUTE ON FUNCTION SCHEMA_CATALOG.pg_name_with_suffix(text, text) TO prom
-- full name doesn't fit, generates a new unique name.
-- Note that there cannot be a collision betweeen a user
-- defined name and a name with a suffix because user
-- defined names of length 63 always get a suffix and
-- conversely, all names with a suffix are length 63.
-- defined names of length 62 always get a suffix and
-- conversely, all names with a suffix are length 62.

-- We use a max name length of 62 not 63 because table creation creates an
-- array type named `_tablename`. We need to ensure that this name is
-- unique as well, so have to reserve a space for the underscore.
CREATE OR REPLACE FUNCTION SCHEMA_CATALOG.pg_name_unique(
full_name_arg text, suffix text)
RETURNS name
AS $func$
SELECT CASE
WHEN char_length(full_name_arg) < 63 THEN
WHEN char_length(full_name_arg) < 62 THEN
full_name_arg::name
ELSE
SCHEMA_CATALOG.pg_name_with_suffix(
Expand Down
4 changes: 4 additions & 0 deletions pkg/pgmodel/pgx.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,10 @@ func (p *pgxInserter) createMetricTable(metric string) (string, error) {
var tableName string
defer res.Close()
if !res.Next() {
err = res.Err()
if err != nil {
return "", err
}
return "", errMissingTableName
}

Expand Down

0 comments on commit 4843f8c

Please sign in to comment.