Fix influx response format #5932
Conversation
| @@ -138,6 +138,7 @@ core|default|role|Option values, `Mixed/Receiver/Aggregator`. **Receiver** mode | |||
| | - | - | database | Database of InfluxDB. | SW_STORAGE_INFLUXDB_DATABASE | skywalking | | |||
| | - | - | actions | The number of actions to collect. | SW_STORAGE_INFLUXDB_ACTIONS | 1000 | | |||
| | - | - | duration | The time to wait at most (milliseconds). | SW_STORAGE_INFLUXDB_DURATION | 1000| | |||
| | - | - | responseFormat | Influxdb response format option. | SW_STORAGE_INFLUXDB_RESPONSE_FORMAT | MSGPACK| | |||
There was a problem hiding this comment.
What options do they have? Could you add a reference link?
There was a problem hiding this comment.
There was a problem hiding this comment.
I find some issues may affect the SkyWalking, such as
JSON does not support integers above 2^53
SkyWalking has many long fields, which in Java could be 2^63-1
There was a problem hiding this comment.
@dmsolr told me, there would be issues when using JSON. Same reason as the above number format issue.
wu-sheng
added
backend
|
One question for the context about the repo, https://github.com/toni-moreno/influxdb-srelay. Is this under maintaining fork version? The reason I am asking is, as an Apache project, we usually only support widely used thing, the 2 PRs you submitted are acceptable, so we could merge. But if this kind of thing keeps happening, I have concerns we will face a conflict. Could you explain more to make us following the context,? |
|
https://github.com/influxdata/influxdb-relay is not a cluster solution,we used for a year and encountered many serious problems (eg two influx nodes frequent data loss,if one node breakdown then start,data will be permanently missing ). |
My point, and concerns from ASF TLP, are both whether the project you choose is really widely used? And who and which community could make the project usable and maintainable for a long time? |
Codecov Report
@@ Coverage Diff @@
## master #5932 +/- ##
============================================
- Coverage 51.19% 46.17% -5.02%
+ Complexity 3477 3081 -396
============================================
Files 1661 881 -780
Lines 35497 21903 -13594
Branches 3901 2124 -1777
============================================
- Hits 18171 10114 -8057
+ Misses 16417 10933 -5484
+ Partials 909 856 -53 Continue to review full report at Codecov.
|
|
yeah,thanks for your suggestion.before skywalking project,we used prometheus with influxdb many years.during this period,influxdb-relay encountered many serious problems.we hava to look for alternatives solution(uber/m3,victoriaMetrics,srelay).as your point,srelay maintenance is a important issue.there are not many maintainers of the project so far.but influxdb with srelay is a reasonable solution util now.this solution also presented to the community.prometheus has many reasonable solution like m3,victoriaMetrics,thanos.we tried various skywalking storage plugins.tsdb is the most reasonable option so far. |
For Prometheus, yes, TSDB makes sense. But honestly, I am not sure whether TSDB is the best option for SkyWalking. |
This would be a big issue, we can't accept to make all things configurable but just for a lack of maintenance project. |
|
not only srelay use JSON format as response format.native influxdb also support JSON format. |
|
elasticsearch is not a reasonable solution for skywalking.agent may push many metrics,each metric has a indics with timestamp 'yyyy-mm-dd'.so many indics and shards that elasticsearch can not manage.on the other hand,the reasonable size of shards in es is 20G-50G. and skywalking storage size is difficult to estimate.how to set configuration of elasticsearch index? |
I am not sure why you say ES can't manage them. 100+TB even PB+ level storages are common the user environments, as far as I know. They may try to find the way to make the balance better in the cluster, but don't have the management issue.
I was not blaming using JSON. I know InfluxDB is supporting it. My feedback is from the original author of this feature, @dmsolr , he told me there would be issues using this format. |
|
@dmsolr hello,we deploy skywalking with influxdb response json format,but no any error. |
|
Hi @zhangjianweibj , thanks! |
|
@zhangjianweibj Could you provide a recorded video to verify all things work? We have several e2e, but they don't cover 100% cases. |
|
unit tests and integration test are designed to check for such problems.as a apache project,it is hard to accept checking this feature by a recorded video. |
|
Sorry, we have to check. Manual tests are always the last option. We have thousands of tests, which definitely helps but we can't say it is perfect. Honestly, I don't understand why a recorded video is an issue, as it is an easy way to check all UI components work well. If we asked you to fill all missing tests in the InfluxDB cases, you would have to suffer more workloads. |
Fix #5931
CHANGESlog.