Skip to content

Conversation

@holiman
Copy link
Contributor

@holiman holiman commented Mar 21, 2023

This is an alternative to #26928 . Instead of upgrading the whole influx package, it switches to use the smaller influxdata/influxdb1-client package.

For influxdb-v2, we already use the smaller client package, so this change removes the large dependency.

There are some api-changes in the handlng of points batch, and this PR needs to be tested in prod.

@holiman
Copy link
Contributor Author

holiman commented Mar 21, 2023

I used this PR to run geth --dev mode for a couple of minutes, shooting the metrics to grafana.

https://geth-bench.ethdevops.io/d/QC1Arp5Wk/single-geth?orgId=1&var-host=martin_laptop&var-percentile=50&from=1679392506844&to=1679392639080

No obvious errors

@fjl fjl self-requested a review March 21, 2023 13:55
@holiman
Copy link
Contributor Author

holiman commented Mar 23, 2023

Swapping this in on bench07 now

@holiman
Copy link
Contributor Author

holiman commented Mar 23, 2023

Everything LGTM (15 minutes with a different PR, and 15 minutes with this PR: https://geth-bench.ethdevops.io/d/Jpk-Be5Wk/dual-geth?var-exp=bench07&var-master=bench07&var-percentile=50&from=1679568700769&to=1679570615983 )

client *client.Client
client client.Client

cache map[string]int64
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this field is not used anymore

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean? It's used in both Ping and send

		case <-pingTicker.C:
			_, _, err := r.client.Ping(0)
	return r.client.Write(bps)

@holiman
Copy link
Contributor Author

holiman commented Mar 23, 2023

Closed in favour of #26963

@holiman holiman closed this Mar 23, 2023
@holiman holiman deleted the influxdb_up branch March 23, 2023 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants