influxdata / influxdb-comparisons

Code for comparison write ups of InfluxDB and other solutions
MIT License
308 stars 112 forks source link

HTTP queries for InfluxDB2 should never be GET #181

Closed codyshepherd closed 3 years ago

codyshepherd commented 3 years ago

This should alleviate a bug in perf tests wherein HTTP queries in 2.x return 405 because they are submitting GETs instead of POSTs

williamhbaker commented 3 years ago

General question...are the InfluxQL queries going to the legacy /query endpoint? For example, a query/URL like this works fine for me with a GET against a 2.0 instance:

http://localhost:8086/query?db=example-db&q=SHOW+FIELD+KEYS+FROM+%22example_measurement%22+LIMIT+200

POST works fine for this too admittedly. From what I can tell the 2.0 influxql handler doesn't care about the method at all. Not sure if there's some other query or path that the 405 is coming from though.

codyshepherd commented 3 years ago

@wbaker85 So it appears from the log output that it's the api/v2/query url path that's being hit, although the influxdb2 docs seem to indicate that only POST is accepted at either endpoint when dealing with 2.x.

Here is the exact log output produced, it seems to indicate that the API is expecting a POST:

+ /root/go/bin/bin/query_benchmarker_influxdb --urls=http://localhost:8086 --debug=0 --print-interval=0 --workers=2 --json=true --organization=example_org --token=token
+ jq . += {branch: "master", commit: "035e11e3ca9ad4a876ad7b810ee70ace84f69d76", time: "1626979199", i_type: "r5.2xlarge", query_format: "http.txt"}
2021/07/22 19:42:51 daemon URLs: [http://localhost:8086]
2021/07/22 19:42:51 Using HTTP client: fast
2021/07/22 19:42:51 Using InfluxDB API version 2
2021/07/22 19:42:51 Trend statistics using 30 samples
2021/07/22 19:42:51 Reading queries to buffer 
2021/07/22 19:42:51 Reading queries done
2021/07/22 19:42:51 Started querying with 2 workers
2021/07/22 19:42:51 Error during request of query ID: 0, HumanLabel: InfluxDB (InfluxQL) max cpu, rand    8 hosts, rand 1h0m0s by 1m, HumanDescription: InfluxDB (InfluxQL) max cpu, rand    8 hosts, rand 1h0m0s by 1m: 2018-01-01T06:43:41Z, Method: GET, Path: /api/v2/query?orgID=6dbb6510bd5671c2, Body:: Invalid write response (status 405): {"code":"method not allowed","message":"allow: OPTIONS, POST"}
codyshepherd commented 3 years ago

I'm not sure what accounts for the discrepancy in the behavior we are seeing. Are you submitting requests against the very latest build of 2.0? Maybe something has changed in the API, as the query tests did seem to work at some point in the past, though I don't have records of exactly when.

williamhbaker commented 3 years ago

I'm using the latest builds, but yeah /api/v2/query won't take anything but a POST. /query will take anything - I think the API docs are wrong, and they conflict with some of the docs like this: https://docs.influxdata.com/influxdb/v2.0/query-data/influxql/#query-a-mapped-bucket-with-influxql

Anyway, I'm wondering more along the lines of: Should InfluxQL queries against a 2.x backend use the /query endpoint instead of the /api/v2/query endpoint? That might be overkill though. Have you been able to confirm that with this change the queries will run successfully? That's probably good enough for now if so.

codyshepherd commented 3 years ago

I haven't run the queries generated by these changes in a perftest, but I have grep-ed through the query file produced by these changes and confirmed that no GETs are present, where they were before these changes. I can do more thorough testing if you prefer.

I feel like if we are trying to benchmark the performance of 2.x, we probably want to first and foremost benchmark the "flagship" endpoint? I don't feel strongly about that though

williamhbaker commented 3 years ago

I'm good with it if you can confirm that the generated queries work! I wouldn't think which endpoint is used would make much difference perf-wise as long as they can handle the input.

codyshepherd commented 3 years ago

So after trying the above changes in a perftest, I encounter a new error. I'll continue to debug:

 format=http.txt
+ cat /mnt/ramdisk/query-devops-scalevar-50-seed-1627488235-http.txt
+ /root/go/bin/bin/query_benchmarker_influxdb --debug=3 --urls=http://localhost:8086 --debug=0 --print-interval=0 --workers=2 --json=true --organization=example_org --token=token
+ jq . += {branch: "perftest-checkout-alt-branch-2.0", commit: "d98004e090f7c9ae6d0703bf41a06cb2f7a44d62", time: "1627488235", i_type: "r5.2xlarge", query_format: "http.txt"}
2021/07/28 16:05:26 daemon URLs: [http://localhost:8086]
2021/07/28 16:05:26 Using HTTP client: fast
2021/07/28 16:05:26 Using InfluxDB API version 2
2021/07/28 16:05:26 Trend statistics using 30 samples
2021/07/28 16:05:26 Reading queries to buffer 
2021/07/28 16:05:26 Reading queries done
2021/07/28 16:05:26 Started querying with 2 workers
2021/07/28 16:05:26 Error during request of query ID: 0, HumanLabel: InfluxDB (InfluxQL) max cpu, rand    8 hosts, rand 1h0m0s by 1m, HumanDescription: InfluxDB (InfluxQL) max cpu, rand    8 hosts, rand 1h0m0s by 1m: 2018-01-01T20:47:11Z, Method: POST, Path: /api/v2/query?orgID=9e2f3dee1b9bff31, Body:SELECT max(usage_user) from cpu where (hostname = 'host_46' or hostname = 'host_6' or hostname = 'host_22' or hostname = 'host_38' or hostname = 'host_24' or hostname = 'host_30' or hostname = 'host_29' or hostname = 'host_16') and time >= '2018-01-01T20:47:11Z' and time < '2018-01-01T21:47:11Z' group by time(1m): Invalid write response (status 400): {"code":"invalid","message":"compilation failed: error at @1:33-1:228: expected comma in property list, got ASSIGN"}
codyshepherd commented 3 years ago

The issue this PR is trying to fix may be addressed by #186