goose icon indicating copy to clipboard operation
goose copied to clipboard

Add replication support for ClickHouse state table + Fixes ListMigration query for ClickHouse

Open arunk-s opened this issue 2 years ago • 8 comments

This MR adds replication support for state table (goose_db_version) in ClickHouse.

  • Replication support can be enabled by calling AttachOptions to goose and provide additional parameters.
  • AttachOptions allows for free form variables to be passed to applicable drivers and currently only ClickHouse dialect inspects it.
  • ClickHouse dialect looks for ON_CLUSTER key present in the options and if enabled picks the cluster name provided or uses a {cluster} macro that is expected to be present in ClickHouse configuration.
  • Changes the table engine to KeeperMap which is a special table engine based on ClickHouse keeper that provides linearizable reads and consistent reads. This was picked to ensure that concurrent writes to state table have deterministic behavior. ClickHouse configuration allows for a strict mode that can ensure that migrations are only applied once.
  • Fixes a bug introduced with https://github.com/pressly/goose/pull/240 which breaks AllowMissingMigrations for Clickhouse.
  • Changes timestamp column type to DateTime64 with nano seconds in precision to ensure as deterministic results as possible when returning rows for ListMigrations.
  • End to End tests are modified to include a test for Cluster mode and additional configuration is added to other tests as well to test with KeeperMap engine.
  • Addes testcontainers-go and google-cmp as dependencies for end to end tests. Testcontainers offers much better API for providing disk mounts and ensuring cleanup.
  • Bumps Go version to 1.20.

A fork of this library with these changes lives at https://gitlab.com/gitlab-org/opstrace/goose which is used in https://gitlab.com/gitlab-org/opstrace/opstrace.

arunk-s avatar May 16 '23 10:05 arunk-s

@mfridman Huge thanks for maintaining this library.

I'm hoping to contribute the fixes back so if you can take some time to review and provide feedback. That will be great :)

arunk-s avatar May 16 '23 10:05 arunk-s

@mfridman Huge thanks for maintaining this library.

I'm hoping to contribute the fixes back so if you can take some time to review and provide feedback. That will be great :)

Thanks, it'll take me some time to get through this one. And I'm also not sure if the /v3 goose module is the right place for this change. (Most of my recent efforts have been on /v4).

Having said that, I'll drop a few comments to get the conversation started.

mfridman avatar May 17 '23 13:05 mfridman

Dropping a thought here. We're soon (#379) going to add a goose provider, which unlocks new capabilities.

One option available to us is exposing the "sql store" interface so users could bring their own. The goose package offers a few default ones, but for more advanced use cases you can implement a thin interface while goose handles the migration logic.

Let me know what you think, as that would satisfy a lot of requests like this one.

Example

The "store" (unsure if that'll be the final name) is the interface goose leverges for all it's migration logic, and currently all implementations are internal: sqlite3, postgres, mysql, clickhouse, etc.

type customStore struct{}

var _ Store = (*customStore)(nil)

func (s *customStore) CreateVersionTable(ctx context.Context, db sqlextended.DBTxConn) error {
	return errors.New("not implemented")
}

func (s *customStore) InsertOrDelete(ctx context.Context, db sqlextended.DBTxConn, direction bool, version int64) error {
	return errors.New("not implemented")
}

func (s *customStore) GetMigration(ctx context.Context, db sqlextended.DBTxConn, version int64) (*GetMigrationResult, error) {
	return nil, errors.New("not implemented")
}

func (s *customStore) ListMigrations(ctx context.Context, db sqlextended.DBTxConn) ([]*ListMigrationsResult, error) {
	return nil, errors.New("not implemented")
}

The sqlextended is an interface to combine *sql.DB, *sql.TX and *sql.Conn.

type DBTxConn interface {
	ExecContext(ctx context.Context, query string, args ...any) (sql.Result, error)
	QueryContext(ctx context.Context, query string, args ...any) (*sql.Rows, error)
	QueryRowContext(ctx context.Context, query string, args ...any) *sql.Row
}

var (
	_ DBTxConn = (*sql.DB)(nil)
	_ DBTxConn = (*sql.Tx)(nil)
	_ DBTxConn = (*sql.Conn)(nil)
)

And with the new goose provider, we could have something like:

store := yourpkg.NewStore(...)

goose.NewProvider(goose.DialectCustom, db, fsys,
	provider.WithCustomStore(store),
)

By setting the dialect to DialectCustom and supplying your own "custom store" you unlock this capability.

Do you think that would satisfy your requirement?

mfridman avatar Oct 23 '23 13:10 mfridman

@mfridman

Yeah I think having a custom store interface can satisfy a lot of these changes. However there can be still some configuration options that are specific to the store, how do you see those options attached to the internal implementation via goose ?

I still think there will need to be a helper function that can be called from the goose that passes arguments or options to the internal implementation. In this MR that role is fulfilled by AttachOptions(map[string]string).

arunk-s avatar Oct 24 '23 19:10 arunk-s

I still think there will need to be a helper function that can be called from the goose that passes arguments or options to the internal implementation. In this MR that role is fulfilled by AttachOptions(map[string]string).

Maybe not? Most (if not all) options can be defined when constructing the Provider. And if you're able to pass a custom Store implementation to the Provider, then you can also initialize it with whatever options.

In this MR you implemented the Querier, but what I'm suggesting is you instead implement this thin interface:

type Store interface {
	CreateVersionTable(ctx context.Context, db sqlextended.DBTxConn) error
	InsertOrDelete(ctx context.Context, db sqlextended.DBTxConn, direction bool, version int64) error
	GetMigration(ctx context.Context, db sqlextended.DBTxConn, version int64) (*GetMigrationResult, error)
	ListMigrations(ctx context.Context, db sqlextended.DBTxConn) ([]*ListMigrationsResult, error)
}

And then in your implementation / code you'd have:

type CustomStore struct {
	OnCluster    bool
	ClusterMacro string
}

var _ Store = (*CustomStore)(nil)

func NewCustomStore(onCluster bool, clusterMacro string) *CustomStore {
	return &CustomStore{
		OnCluster:    onCluster,
		ClusterMacro: clusterMacro,
	}
}

func (c *CustomStore) CreateVersionTable(context.Context, sqlextended.DBTxConn) error {
	return errors.New("not implemented")
}

func (c *CustomStore) InsertOrDelete(context.Context, sqlextended.DBTxConn, bool, int64) error {
	return errors.New("not implemented")
}

func (c *CustomStore) GetMigration(context.Context, sqlextended.DBTxConn, int64) (*GetMigrationResult, error) {
	return nil, errors.New("not implemented")
}

func (c *CustomStore) ListMigrations(context.Context, sqlextended.DBTxConn) ([]*ListMigrationsResult, error) {
	return nil, errors.New("not implemented")
}

And then when you start your app, you would:

func main() {
	// Bring your own Store implementation:
	// 	- CreateVersionTable
	// 	- InsertOrDelete
	// 	- GetMigration
	// 	- ListMigrations
	store := NewCustomStore(true, "clusterMacro")
	p, err := provider.NewProvider(CustomDialect, db, fsys, WithCustomStore(store))
}

Based on registered migrations, goose knows how to use the 4 store methods to do various migration-related tasks.

mfridman avatar Oct 25 '23 13:10 mfridman

I like your proposal @mfridman ! The only drawback I see is that it cannot be used with the "standard" goose binary. But as I already use a custom one, it's no big deal for me.

Also, a bonus point would be that the custom store could access the regular store to decorate it instead of rewriting all the methods.

iamluc avatar Oct 25 '23 15:10 iamluc

Based on registered migrations, goose knows how to use the 4 store methods to do various migration-related tasks.

@mfridman, I see. Yeah that might work. I can try to give it a go but I am not yet sure when I'll be able to get to it.

arunk-s avatar Oct 26 '23 16:10 arunk-s

Based on registered migrations, goose knows how to use the 4 store methods to do various migration-related tasks.

@mfridman, I see. Yeah that might work. I can try to give it a go but I am not yet sure when I'll be able to get to it.

No worries whatsoever, the provider API is still internal and under active development. It'll land sometime in the next few weeks with a blog post about what's new and the more advanced use cases. I think it'll open a lot of opportunities for us to build upon.

mfridman avatar Oct 26 '23 16:10 mfridman