search

jaegertracing / jaeger-clickhouse

Jaeger ClickHouse storage plugin implementation
Apache License 2.0
236 stars 50 forks source link

Added out-of-box replication support #53

Closed EinKrebs closed 2 years ago

EinKrebs commented 2 years ago

@pavolloffay maybe we should use 2 config options:

This will make things much easier.

pavolloffay commented 2 years ago

That does not make sense to me. The the common use case is to create tables in the database where you are connecting. Users could also specify the same database for both.

EinKrebs commented 2 years ago

@pavolloffay So, is the current approach in PR okay?

pavolloffay commented 2 years ago

No we cannot use script that uses USE because of https://github.com/go-sql-driver/mysql/issues/173#issuecomment-28539448.

We can make a requirement for user to create a database before starting jaeger, however we need to change the db name in scripts as I mentioned before.

EinKrebs commented 2 years ago

No we cannot use script that uses USE because of go-sql-driver/mysql#173 (comment).

We can make a requirement for user to create a database before starting jaeger, however we need to change the db name in scripts as I mentioned before.

@pavolloffay Then maybe we should keep "default" as the default database name. In that case we could get rid of our complex two-step connection.

pavolloffay commented 2 years ago

Can you explain why? I think it's still useful for quick demos as default.

EinKrebs commented 2 years ago

Can you explain why? I think it's still useful for quick demos as default.

What to explain?

pavolloffay commented 2 years ago

Why the default database cannot be used :)

EinKrebs commented 2 years ago

It sure can, and I want it to be used. But from your guide to sharding and replication I thought that we should create tables in jaeger database. Now I see you're okey with using default as the default database name. Gonna change it in PR.

EinKrebs commented 2 years ago

@pavolloffay All done, I guess.

pavolloffay commented 2 years ago

Did you test this with a real CH server?

pavolloffay commented 2 years ago

Also note that it should work for any database name that is in the config.

EinKrebs commented 2 years ago

Did you test this with a real CH server?

Yes, I did, with custom database names as well as custom table names.

pavolloffay commented 2 years ago

Great work @EinKrebs

EinKrebs commented 2 years ago

Great work @EinKrebs

Thanks a lot.