[GitHub] [skywalking] dmsolr commented on a change in pull request #5637: Health check for InfluxDB

[GitBox <gi...@apache.org>]

dmsolr commented on a change in pull request #5637:
URL: https://github.com/apache/skywalking/pull/5637#discussion_r503198541



##########
File path: oap-server/server-storage-plugin/storage-influxdb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/influxdb/InfluxClient.java
##########
@@ -101,11 +113,10 @@ private InfluxDB getInflux() {
 
         try {
             QueryResult result = getInflux().query(new Query(query.getCommand()));
-            if (result.hasError()) {
-                throw new IOException(result.getError());
-            }
+            healthChecker.health();

Review comment:
       hi @xbkaishui 
   I had test some cases. The result show following
   1. syntax error/parse error, connection, etc will throw an exception.
   2. aggregate function error maybe doesn't throw.
   
   For example:
   1. case 1
    ```
   Query: SELECT mean("square"),"round" FROM "peg"
   Response: QueryResult [results=[Result [series=null, error=mixing aggregate and non-aggregate queries is not supported]], error=null]
   ```
   
   2. case 2 
   ```
   Query: SELECT sum("square", 2),"round" FROM "peg"
   
   Response: [results=[Result [series=null, error=invalid number of arguments for mean, expected 1, got 2]], error=null]
   ```
   
   These two examples put error messages in clauses only. :( 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org