search

omniosorg / omnios-extra

Packages for OmniOS extra
https://omnios.org
Other
27 stars 58 forks source link

Add VictoriaMetrics package, a time series database. #1512

Closed gkoh closed 1 month ago

gkoh commented 2 months ago

Package multiple pieces of the Prometheus compatible VictoriaMetrics suite:

Whilst here, correct a minor spelling error in functions.sh.

gkoh commented 2 months ago

I need some advice on this package, there are some oddities.

  1. I have claimed uid/gid 93, hope this is correct

  2. Configuration is not via file, it is via command-line argument or equivalent envvar. Updating the service manifest to change the command line is crazy, so I think the right answer is by applying service profiles that modify the instance method_environment. I have placed examples of such profiles in share/victoriametrics/xxx-profile.xml.

  3. Using an envvar, I have set the storageDataPath for the server victoria-metrics in the default svc manifest, however I believe this will be overwritten by an update. This should probably be applied via profile only. By doing so, the server and agent will require a service profile to be applied or they will not function correctly.

  4. I have added some install notes to cover the profile stuff above, I hope this is sufficient.

gkoh commented 2 months ago

I need some advice on this package, there are some oddities.

  1. I have claimed uid/gid 93, hope this is correct

Yes, and thank you for updating the idlist file. That's where we track allocated UIDs/GIDs across all repositories. Please could you use tabs instead of spaces so that the new lines match the existing ones? Same in packages.md.

Done.

  1. Using an envvar, I have set the storageDataPath for the server victoria-metrics in the default svc manifest, however I believe this will be overwritten by an update. This should probably be applied via profile only. By doing so, the server and agent will require a service profile to be applied or they will not function correctly.

Have you checked if they are overwritten on update? I don't think they will be if they have ever been modified with svccfg or a profile.

OK, I checked again, properly. You are correct. On update, values modified by a profile are not overwritten. Thus, I have left it as-is.

  1. I have added some install notes to cover the profile stuff above, I hope this is sufficient.

Yes, thank you!

Please could you also update doc/baseline.md?

I updated both doc/baseline and doc/baseline.aarch64. I can't test an aarch64 build (no hardware) but VictoriaMetrics themselves release arch64 binaries.

Thank you for the review.

gkoh commented 2 months ago

Apologies, github didn't like my fat-fingered rebase so some of the review comment context got nuked. I think I have addressed all review comments.

hadfl commented 1 month ago

I am very sorry, I messed up when rebasing this which closed the PR. I have re-opened it https://github.com/omniosorg/omnios-extra/pull/1531