clickhouse-grafana icon indicating copy to clipboard operation
clickhouse-grafana copied to clipboard

Allow setup adhoc queries in ConnectionSettings

Open asafcombo opened this issue 4 years ago • 10 comments

There are 2 performance issues with how adhoc query gets the values.

https://github.com/Vertamedia/clickhouse-grafana/blob/d92ff14d529f5e02e835860adf4fa4d58192ed4b/src/adhoc.ts#L3

const valuesQuery = "SELECT DISTINCT {field} AS value FROM {database}.{table} LIMIT 300";

issue 1

Happens when cardinality of values is lower than 300. In this case, the engine will scan the entire table to try and get to 300 values.

I suggest modify to

"SELECT DISTINCT value FROM ( SELECT  {field} AS value FROM {database}.{table}  LIMIT 100000 ) LIMIT 300";

The query above limits the total amount of data to scan.

issue2

The 2nd issue happens with tiered storage setups. The scan is not limited to fast tiered drives. Usually the latest data is located on fast disks so I propose to do

"SELECT DISTINCT value FROM ( SELECT  {field} AS value FROM {database}.{table} WHERE  timestamp >= now() - INTERVAL 30 DAY LIMIT 100000 ) LIMIT 300";

Although the above case is very specifc to the timestamp field name.

asafcombo avatar Mar 25 '21 14:03 asafcombo

@asafcombo thanks a lot for the suggestions, look like to resolve issue2, we should allow configuring valuesQuery, cause timestamp field name is very specific, I will think about it, feel free to make pull request

Slach avatar Mar 25 '21 15:03 Slach

if valuesQuery could be configured, it could also allow a more fine grained control on the LIMITS, and could also make the adhoc_query_filter obsolete

asafcombo avatar Mar 30 '21 12:03 asafcombo

We could also limit the value to the time interval of the dashboard itself? Could we leverage variable like $from and $to here?

dsztykman avatar Sep 05 '21 00:09 dsztykman

@dsztykman I think if we use $from and $to we need also know which field is defined as "timestamp" field on specified {database}.{table}, but currently ad-hoc interface doesn't have ability to specified, we can try to parse system.columns before make valuesQuery

Slach avatar Sep 05 '21 15:09 Slach

@dsztykman feel free to make pull request if you figure-out with plugin source

Slach avatar Sep 05 '21 15:09 Slach

I'm going to try replacing with: const valuesQuery = "SELECT DISTINCT {field} AS value FROM {database}.{table} WHERE $timeFilter LIMIT 300";

dsztykman avatar Sep 05 '21 16:09 dsztykman

@dsztykman any progress with this approach?

Slach avatar Nov 18 '21 16:11 Slach

@Slach I actually have some issues testing, which aren't related:

frontend_builder_1   | ++ ls -la ./node_modules/
frontend_builder_1   | + [[ 3 == 3 ]]
frontend_builder_1   | + npm install --production=false
frontend_builder_1   | npm notice 
frontend_builder_1   | npm notice New patch version of npm available! 8.1.0 -> 8.1.4
frontend_builder_1   | npm notice Changelog: <https://github.com/npm/cli/releases/tag/v8.1.4>
frontend_builder_1   | npm notice Run `npm install -g [email protected]` to update!
frontend_builder_1   | npm notice 
frontend_builder_1   | npm ERR! code ERESOLVE
frontend_builder_1   | npm ERR! ERESOLVE unable to resolve dependency tree
frontend_builder_1   | npm ERR! 
frontend_builder_1   | npm ERR! While resolving: [email protected]
frontend_builder_1   | npm ERR! Found: [email protected]
frontend_builder_1   | npm ERR! node_modules/typescript
frontend_builder_1   | npm ERR!   dev typescript@"^3.9.5" from the root project
frontend_builder_1   | npm ERR! 
frontend_builder_1   | npm ERR! Could not resolve dependency:
frontend_builder_1   | npm ERR! peer typescript@"^2.4.0" from [email protected]
frontend_builder_1   | npm ERR! node_modules/plugin-typescript
frontend_builder_1   | npm ERR!   dev plugin-typescript@"^8.0.0" from the root project
frontend_builder_1   | npm ERR! 
frontend_builder_1   | npm ERR! Fix the upstream dependency conflict, or retry
frontend_builder_1   | npm ERR! this command with --force, or --legacy-peer-deps
frontend_builder_1   | npm ERR! to accept an incorrect (and potentially broken) dependency resolution.
frontend_builder_1   | npm ERR! 
frontend_builder_1   | npm ERR! See /root/.npm/eresolve-report.txt for a full report.
frontend_builder_1   | 
frontend_builder_1   | npm ERR! A complete log of this run can be found in:
frontend_builder_1   | npm ERR!     /root/.npm/_logs/2021-11-19T15_55_18_635Z-debug.log
clickhouse-grafana_frontend_builder_1 exited with code 1

dsztykman avatar Nov 19 '21 15:11 dsztykman

@dsztykman could you run:

git remote add altinity [email protected]:Altinity/clickhouse-grafana.git
git pull altinity master

I added package-lock.json on forked repo, and fix this depencency

Slach avatar Nov 19 '21 17:11 Slach

Unfortunately what I'm doing doesn't work... On the principle I wanted to create a new variable similar to: https://github.com/Vertamedia/clickhouse-grafana/blob/d92ff14d529f5e02e835860adf4fa4d58192ed4b/src/datasource.ts#L11

I'm trying to do a variable called adhoc_value_filter which the user could override. And user could provide the value like that: SELECT DISTINCT {field} AS value FROM {database}.{table} WHERE timestamp > $from and timestamp < $to LIMIT 300";

But I really can't make it work sorry

dsztykman avatar Nov 26 '21 15:11 dsztykman