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

Commit

Permalink
Disable synchronous_commit by default for writer connections
Browse files Browse the repository at this point in the history
Disabling synchronous_commit for the database connections in the writer pool
will improve performance. The tradeoff is that some amount of writes may be lost
in the event of a crash. This is likely an "okay" tradeoff for monitoring data.
By default, we now disable synchronous_commit at the session level on connections
in the writer pool. This does not impact other connections/sessions. A new config
option has been added to override this default.
  • Loading branch information
jgpruitt committed Jul 22, 2022
1 parent f153d24 commit 0e9c66a
Show file tree
Hide file tree
Showing 5 changed files with 146 additions and 48 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -22,6 +22,7 @@ We use the following categories for changes:
- Deprecate `openTelemetry` and `prometheus` top-level objects in helm chart in favour of `.service.openTelemetry` and `.service.prometheus` [#1495]
- Allow disabling exposition of promscale port in Service object. [#1495]
- Enable prometheus annotation based scraping only when ServiceMonitor is disabled. [#1495]
- `db.connections.writer-pool.synchronous-commit` controls whether synchronous_commit is enabled/disabled for writer database connections. [#1499]

### Changed
- `db.num-writer-connections` now sets the absolute number of write connections for writing metrics. [#1430]
Expand Down
35 changes: 18 additions & 17 deletions docs/configuration.md
Expand Up @@ -59,23 +59,24 @@ The following subsections cover all CLI flags which promscale supports. You can

### Database flags

| Flag | Type | Default | Description |
|---------------------------------|:--------:|:------------------------------:|:-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| db.app | string | promscale@{version} | 'app' sets application_name in database connection string. This is helpful during debugging when looking at pg_stat_activity. |
| db.connection-timeout | duration | 60 seconds | Timeout for establishing the connection between Promscale and TimescaleDB. |
| db.connections-max | integer | 80% of possible connections db | Maximum number of connections to the database that should be opened at once. It defaults to 80% of the maximum connections that the database can handle. |
| db.connections.num-writers | integer | 0 | Number of database connections for writing metrics to database. By default, this will be set based on the number of CPUs available to the DB Promscale is connected to. |
| db.connections.reader-pool.size | integer | 30% of possible connections db | Maximum size of the reader pool of database connections. This defaults to 30% of max_connections allowed by the database. |
| db.connections.writer-pool.size | integer | 50% of possible connections db | Maximum size of the writer pool of database connections. This defaults to 50% of max_connections allowed by the database. |
| db.host | string | localhost | Host for TimescaleDB/Vanilla Postgres. |
| db.name | string | timescale | Database name. |
| db.password | string | | Password for connecting to TimescaleDB/Vanilla Postgres. |
| db.port | int | 5432 | TimescaleDB/Vanilla Postgres connection password. |
| db.read-only | boolean | false | Read-only mode for the connector. Operations related to writing or updating the database are disallowed. It is used when pointing the connector to a TimescaleDB read replica. |
| db.ssl-mode | string | require | TimescaleDB/Vanilla Postgres connection ssl mode. If you do not want to use ssl, pass `allow` as value. |
| db.statements-cache | boolean | true | Whether database connection pool should use cached prepared statements. Disable if using PgBouncer. |
| db.uri | string | | TimescaleDB/Vanilla PostgresSQL URI. Example:`postgres://postgres:password@localhost:5432/timescale?sslmode=require` |
| db.user | string | postgres | TimescaleDB/Vanilla Postgres user. |
| Flag | Type | Default | Description |
|-----------------------------------------------|:--------:|:------------------------------:|:-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| db.app | string | promscale@{version} | 'app' sets application_name in database connection string. This is helpful during debugging when looking at pg_stat_activity. |
| db.connection-timeout | duration | 60 seconds | Timeout for establishing the connection between Promscale and TimescaleDB. |
| db.connections-max | integer | 80% of possible connections db | Maximum number of connections to the database that should be opened at once. It defaults to 80% of the maximum connections that the database can handle. |
| db.connections.num-writers | integer | 0 | Number of database connections for writing metrics to database. By default, this will be set based on the number of CPUs available to the DB Promscale is connected to. |
| db.connections.reader-pool.size | integer | 30% of possible connections db | Maximum size of the reader pool of database connections. This defaults to 30% of max_connections allowed by the database. |
| db.connections.writer-pool.size | integer | 50% of possible connections db | Maximum size of the writer pool of database connections. This defaults to 50% of max_connections allowed by the database. |
| db.connections.writer-pool.synchronous-commit | boolean | false | Enable/disable synchronous_commit on database connections in the writer pool. |
| db.host | string | localhost | Host for TimescaleDB/Vanilla Postgres. |
| db.name | string | timescale | Database name. |
| db.password | string | | Password for connecting to TimescaleDB/Vanilla Postgres. |
| db.port | int | 5432 | TimescaleDB/Vanilla Postgres connection password. |
| db.read-only | boolean | false | Read-only mode for the connector. Operations related to writing or updating the database are disallowed. It is used when pointing the connector to a TimescaleDB read replica. |
| db.ssl-mode | string | require | TimescaleDB/Vanilla Postgres connection ssl mode. If you do not want to use ssl, pass `allow` as value. |
| db.statements-cache | boolean | true | Whether database connection pool should use cached prepared statements. Disable if using PgBouncer. |
| db.uri | string | | TimescaleDB/Vanilla PostgresSQL URI. Example:`postgres://postgres:password@localhost:5432/timescale?sslmode=require` |
| db.user | string | postgres | TimescaleDB/Vanilla Postgres user. |

### Telemetry flags (for telemetry generated by the Promscale connector itself)

Expand Down
18 changes: 17 additions & 1 deletion pkg/pgclient/client.go
Expand Up @@ -92,7 +92,7 @@ func NewClient(r prometheus.Registerer, cfg *Config, mt tenancy.Authorizer, sche
if err != nil {
return nil, fmt.Errorf("get writer pg-config: %w", err)
}
writerPgConfig.AfterConnect = schemaLocker
SetWriterPoolAfterConnect(writerPgConfig, schemaLocker, cfg.WriterSynchronousCommit)
writerPool, err = pgxpool.ConnectConfig(context.Background(), writerPgConfig)
if err != nil {
return nil, fmt.Errorf("err creating writer connection pool: %w", err)
Expand Down Expand Up @@ -139,6 +139,22 @@ func NewClient(r prometheus.Registerer, cfg *Config, mt tenancy.Authorizer, sche
return client, err
}

func SetWriterPoolAfterConnect(writerPgConfig *pgxpool.Config, schemaLocker LockFunc, synchronousCommit bool) {
if !synchronousCommit {
// if synchronous_commit should be disabled, we use the AfterConnect hook so that we can set it to 'off'
// for the session before the connection is added to the pool for use
writerPgConfig.AfterConnect = func(ctx context.Context, conn *pgx.Conn) error {
_, err := conn.Exec(ctx, "SET SESSION synchronous_commit to 'off'")
if err != nil {
return err
}
return schemaLocker(ctx, conn)
}
} else {
writerPgConfig.AfterConnect = schemaLocker
}
}

func (cfg *Config) getPgConfig(poolSize int) (*pgxpool.Config, error) {
min := MinPoolSize
max := poolSize
Expand Down
63 changes: 33 additions & 30 deletions pkg/pgclient/config.go
Expand Up @@ -21,39 +21,41 @@ import (

// Config for the database.
type Config struct {
CacheConfig cache.Config
AppName string
Host string
Port int
User string
Password string
Database string
SslMode string
DbConnectionTimeout time.Duration
IgnoreCompressedChunks bool
AsyncAcks bool
WriteConnections int
WriterPoolSize int
ReaderPoolSize int
MaxConnections int
UsesHA bool
DbUri string
EnableStatementsCache bool
CacheConfig cache.Config
AppName string
Host string
Port int
User string
Password string
Database string
SslMode string
DbConnectionTimeout time.Duration
IgnoreCompressedChunks bool
AsyncAcks bool
WriteConnections int
WriterPoolSize int
WriterSynchronousCommit bool
ReaderPoolSize int
MaxConnections int
UsesHA bool
DbUri string
EnableStatementsCache bool
}

const (
defaultDBUri = ""
defaultDBHost = "localhost"
defaultDBPort = 5432
defaultDBUser = "postgres"
defaultDBName = "timescale"
defaultDBPassword = ""
defaultSSLMode = "require"
defaultConnectionTime = time.Minute
defaultDbStatementsCache = true
MinPoolSize = 2
defaultPoolSize = -1
defaultMaxConns = -1
defaultDBUri = ""
defaultDBHost = "localhost"
defaultDBPort = 5432
defaultDBUser = "postgres"
defaultDBName = "timescale"
defaultDBPassword = ""
defaultSSLMode = "require"
defaultConnectionTime = time.Minute
defaultDbStatementsCache = true
MinPoolSize = 2
defaultPoolSize = -1
defaultMaxConns = -1
defaultWriterSynchronousCommit = false
)

var (
Expand Down Expand Up @@ -81,6 +83,7 @@ func ParseFlags(fs *flag.FlagSet, cfg *Config) *Config {
"By default, this will be set based on the number of CPUs available to the DB Promscale is connected to.")
fs.IntVar(&cfg.WriterPoolSize, "db.connections.writer-pool.size", defaultPoolSize, "Maximum size of the writer pool of database connections. This defaults to 50% of max_connections "+
"allowed by the database.")
fs.BoolVar(&cfg.WriterSynchronousCommit, "db.connections.writer-pool.synchronous-commit", defaultWriterSynchronousCommit, "Enable/disable synchronous_commit on database connections in the writer pool.")
fs.IntVar(&cfg.ReaderPoolSize, "db.connections.reader-pool.size", defaultPoolSize, "Maximum size of the reader pool of database connections. This defaults to 30% of max_connections "+
"allowed by the database.")
fs.IntVar(&cfg.MaxConnections, "db.connections-max", defaultMaxConns, "Maximum number of connections to the database that should be opened at once. "+
Expand Down
77 changes: 77 additions & 0 deletions pkg/tests/end_to_end_tests/sync_commit_test.go
@@ -0,0 +1,77 @@
package end_to_end_tests

import (
"context"
"testing"

"github.com/jackc/pgx/v4"
"github.com/jackc/pgx/v4/pgxpool"
"github.com/stretchr/testify/require"
"github.com/timescale/promscale/pkg/pgclient"
)

/// TestWriterSynchronousCommit ensures that the database pool for the writer can be configured with synchronous_commit
/// turned on or off and that the effects are limited to that pool
func TestWriterSynchronousCommit(t *testing.T) {

// creates a new pool of database connections as would be created for the writer path
createWriterPool := func(connStr string, lockerCalled *bool, synchronousCommit bool) *pgxpool.Pool {
pgConfig, err := pgxpool.ParseConfig(connStr)
if err != nil {
t.Fatal(err)
}
pgConfig.MaxConns = 2
pgConfig.MinConns = 1
*lockerCalled = false
schemaLocker := func(ctx context.Context, conn *pgx.Conn) error {
*lockerCalled = true
return nil
}
pgclient.SetWriterPoolAfterConnect(pgConfig, schemaLocker, synchronousCommit)
writerPool, err := pgxpool.ConnectConfig(context.Background(), pgConfig)
if err != nil {
t.Fatal(err)
}
return writerPool
}

// gets the value of the synchronous_commit setting from the database
getSynchronousCommit := func(writerPool *pgxpool.Pool) string {
var setting string
err := writerPool.QueryRow(context.Background(), "show synchronous_commit").Scan(&setting)
if err != nil {
t.Fatal(err)
}
return setting
}

withDB(t, "writer_sync_commit", func(db *pgxpool.Pool, t testing.TB) {
var lockerCalled bool // used to ensure that the schema locker function is still called

// create a writer pool with synchronous_commit turned off
writerPool1 := createWriterPool(db.Config().ConnString(), &lockerCalled, false)
setting := getSynchronousCommit(writerPool1)
require.Equal(t, "off", setting, "expected synchronous_commit to be off but it was %s", setting)
require.True(t, lockerCalled, "schemaLocker function should have been called and wasn't")

// ensure that setting synchronous_commit to off on the writer pool did not impact the setting
// in other database sessions
setting = getSynchronousCommit(db)
require.Equal(t, "on", setting, "expected synchronous_commit to be on but it was %s", setting)

// make sure the setting stays off after the first transaction finished
setting = getSynchronousCommit(writerPool1)
require.Equal(t, "off", setting, "expected synchronous_commit to be off but it was %s", setting)
writerPool1.Close()

// now create a writer pool with synchronous_commit turned on
lockerCalled = false
writerPool2 := createWriterPool(db.Config().ConnString(), &lockerCalled, true)

// make sure the setting is on and the schema locker function was called
setting = getSynchronousCommit(writerPool2)
require.Equal(t, "on", setting, "expected synchronous_commit to be on but it was %s", setting)
require.True(t, lockerCalled, "schemaLocker function should have been called and wasn't")
writerPool2.Close()
})
}

0 comments on commit 0e9c66a

Please sign in to comment.