timescale / prometheus-postgresql-adapter

Use PostgreSQL as a remote storage database for Prometheus
Apache License 2.0
335 stars 66 forks source link

Service restart fails #57

Open Knudian opened 5 years ago

Knudian commented 5 years ago

Hello,

While I am trying to use this somehow great tool, I have a problem on making it restart on a virtual machine.

The problem is simple : the adapter wants to recreate the whole database schema, which is a problem IMHO. As the log says : level=error ts=2019-01-16T11:27:24.991560544Z caller=log.go:33 err="pq: relation « metrics_labels » already exists".

I am not fluent enough with go to propose a PR for this, but I think the adapter should only create anything in the table, if the required table/views/seqs are not already existing.

Best regards

iushas commented 5 years ago

I alse have been confuesd by this question for a long time, please help me solve this problem.

iushas commented 5 years ago

I don't want to recreate whole database, otherwise i want keep old data.

Knudian commented 5 years ago

Doesn't this https://github.com/timescale/prometheus-postgresql-adapter/blob/a51ef44a1d9bfb5408530c93e300649d1d5a55bb/postgresql/client.go#L152-L161 close this issue ?


Just read the blame. It's two years old.

Maybe a check on this line to allow a silent fail (since the structure exists) ?

bboule commented 5 years ago

Lets u have a look and get back to the thread

krisavi commented 4 years ago

@Knudian This is a bad approach. It is hardcoded to Englsih.

I am not fluent enough with go to propose a PR for this, but I think the adapter should only create anything in the table, if the required table/views/seqs are not already existing.

I brought it up in other issue #90 as it is quite the same issue but worded differently. Seems like the issue is that it should only create if tables do not exist.

https://github.com/timescale/prometheus-postgresql-adapter/issues/90#issuecomment-577604063 Strings.Contains(err.Error(), "already exists") works only on systems where DB language is English. In my environment I wrote totally different way to check for tables existence. If it returns 0 tables, then it starts the creation script. Without it, it will not try to create, thus not getting to piece of code where it should even throw error about table existing.

Knudian commented 4 years ago

@krisavi : I did make a workaround for this point, not in the adapter (I am still struggling to produce correct tests in golang), but in the psql procedure called.

Since the command uses the create_prometheus_table procedure, I edited the procedure itself, by making it silently fails by adding IF NOT EXISTS to all inner creation statements of it.

Shoud the adapter add a check on validating the schema produced by this command? I don't think so, since it is outside of its scope.

krisavi commented 4 years ago

@Knudian yeah, makes sense. Check in go to query table is not the simplest to implement and to make it so it would not fail whole process if for some reason only one table exists, but rest are missing. In SQL procedure it is easier to fail silently and still continue to check all the other tables.

I did implement something in Go that fetches all the rows from information schema that have name like the table it looks for. Then it iterates row by row to check if they are existing. If all are there, then it will skip the creation process. I know it will fail if first table is there and rest are missing. It would end up initiating creation process, which would still fail.

Trying to make this check in adapter is prone to errors. Even now it is buggy on non-english DBs