Bug 275828
| Summary: | net-mgmt/victoria-metrics: Update to 1.102.12, switch to GO_MODULE and add a cluster version | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | Ports & Packages | Reporter: | Boris Korzun <drtr0jan> | ||||||||||||||||||||
| Component: | Individual Port(s) | Assignee: | Alexey Dokuchaev <danfe> | ||||||||||||||||||||
| Status: | In Progress --- | ||||||||||||||||||||||
| Severity: | Affects Some People | CC: | danfe, fuz, janlugt, lapo, madpilot | ||||||||||||||||||||
| Priority: | --- | Flags: | madpilot:
maintainer-feedback?
(danfe) drtr0jan: maintainer-feedback? (danfe) |
||||||||||||||||||||
| Version: | Latest | ||||||||||||||||||||||
| Hardware: | Any | ||||||||||||||||||||||
| OS: | Any | ||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||
Created attachment 247877 [details] victoria-metrics.patch Update to 1.93.10. Also switched to GO_MODULE framework. Also added a cluster version of victoria-metrics (with vminsert, vmstorage and vmselect). Also added vmalert rc-script for net-mgmt/vmutils. Sanited rc-scripts. Alexey Dokuchaev, what do you think about update to 1.93.10? There's an annoying issue https://github.com/VictoriaMetrics/metrics/issues/59 in 1.93.9. Current 1.93.9 is also incompatibile with Go 1.20 as it uses 1.25.3 of "metrics" dependency with this bug: https://github.com/VictoriaMetrics/metrics/issues/59 SIZE (VictoriaMetrics-metrics-v1.25.3_GH0.tar.gz) = 265801 Which leads to errors like: panic: BUG: unexpected runtimemetrics.KindBad for sample.Name="/gc/gomemlimit:bytes" 1.93.10 fixes this as well by upgrading dependency. maintainer timeout Hi, Since I'm having issues due to the known bug in 1.93.9 I'm going to test this patch. It looks fine, but I have one question for the submitter: why the rc scripts have an unconditional "-loggerDisableTimestamps"? Being syslog not the default output, timestamps by the logger are actually useful. Could the -loggerDisableTimestamps be made conditional on enabling syslog support? BTW thanks a lot for the patch and your work on it! Created attachment 247991 [details] victoria-metrics.patch (In reply to Guido Falsi from comment #4) Good idea! Updated the patch. Can you test/review the feauture? (In reply to Boris Korzun from comment #5) I was going to test a similar modification myself (the name I gave the variable was different though!) Also your previous patch had some plist issues, reported by poudriere. I'll merge patches and test, will file an updated patch once I have verified it. (In reply to Guido Falsi from comment #6) Hmm, I've got a '===> No pkg-plist issues found (check-plist)' message while built by poudriere all three ports... (In reply to Boris Korzun from comment #7) strange here it reported some problems: ====> Running Q/A tests (stage-qa) ====> Checking for pkg-plist issues (check-plist) ===> Parsing plist ===> Checking for items in STAGEDIR missing from pkg-plist Error: Orphaned: %%PORTDOCS%%%%DOCSDIR%%/VictoriaLogs/CHANGELOG.md Error: Orphaned: %%PORTDOCS%%%%DOCSDIR%%/VictoriaLogs/FAQ.md Error: Orphaned: %%PORTDOCS%%%%DOCSDIR%%/VictoriaLogs/LogsQL.md Error: Orphaned: %%PORTDOCS%%%%DOCSDIR%%/VictoriaLogs/QuickStart.md Error: Orphaned: %%PORTDOCS%%%%DOCSDIR%%/VictoriaLogs/README.md Error: Orphaned: %%PORTDOCS%%%%DOCSDIR%%/VictoriaLogs/Roadmap.md Error: Orphaned: %%PORTDOCS%%%%DOCSDIR%%/VictoriaLogs/data-ingestion/Filebeat.md Error: Orphaned: %%PORTDOCS%%%%DOCSDIR%%/VictoriaLogs/data-ingestion/Fluentbit.md Error: Orphaned: %%PORTDOCS%%%%DOCSDIR%%/VictoriaLogs/data-ingestion/Logstash.md Error: Orphaned: %%PORTDOCS%%%%DOCSDIR%%/VictoriaLogs/data-ingestion/Promtail.md Error: Orphaned: %%PORTDOCS%%%%DOCSDIR%%/VictoriaLogs/data-ingestion/README.md Error: Orphaned: %%PORTDOCS%%%%DOCSDIR%%/VictoriaLogs/data-ingestion/Vector.md Error: Orphaned: %%PORTDOCS%%%%DOCSDIR%%/VictoriaLogs/keyConcepts.md Error: Orphaned: %%PORTDOCS%%%%DOCSDIR%%/VictoriaLogs/querying/README.md ===> Checking for items in pkg-plist which are not in STAGEDIR Error: Missing: %%DOCSDIR%%/CNAME Error: Missing: %%DOCSDIR%%/Cluster-VictoriaMetrics.md Error: Missing: %%DOCSDIR%%/Cluster-VictoriaMetrics_cluster-scheme.png Error: Missing: %%DOCSDIR%%/Gemfile Error: Missing: %%DOCSDIR%%/Makefile Error: Missing: %%DOCSDIR%%/_config.yml Error: Missing: %%DOCSDIR%%/_includes/extra/head.html Error: Missing: %%DOCSDIR%%/_includes/extra/script.js Error: Missing: %%DOCSDIR%%/_includes/extra/styles.scss Error: Missing: %%DOCSDIR%%/assets/README.md Error: Missing: %%DOCSDIR%%/assets/css/clipboard.css Error: Missing: %%DOCSDIR%%/assets/images/favicon.svg Error: Missing: %%DOCSDIR%%/assets/images/vm_logo.svg Error: Missing: %%DOCSDIR%%/assets/js/clipboard.min.js Error: Missing: %%DOCSDIR%%/enterprise.md Error: Missing: %%DOCSDIR%%/googlec3812dcf278679ec.html ===> Error: Plist issues found. *** Error code 1 Not sure what could be causing this, maybe something went wrong when I imported the patch. (In reply to Boris Korzun from comment #1) > Also switched to GO_MODULE framework. So I've read about it here: https://reviews.freebsd.org/D28184. I don't see how that's an improvement: 1) it hides the from user some nice to know details about to-be-downloaded dependencies (their quantity, their size), and 2) per the quotes: "unfortunately this change won't make downloads smaller". What's the win here for us? > Also added a cluster version of victoria-metrics (with vminsert, vmstorage, > and vmselect). What's the difference between cluster and regular (non-cluster?) versions of these programs? Do we actually need both? > What do you think about update to 1.93.10? There's an annoying issue... Yes, I'll do it right away. (In reply to Alexey Dokuchaev from comment #9) >> Also switched to GO_MODULE framework. > So I've read about it here: https://reviews.freebsd.org/D28184. I don't see > how that's an improvement: 1) it hides the from user some nice to know details > about to-be-downloaded dependencies (their quantity, their size), and 2) per > the quotes: "unfortunately this change won't make downloads smaller". What's > the win here for us? While I don't really have a strong opinion, an advantage of this is not being throttled by github while trying to run "make gomod-vendor", which happened to me multiple times while trying to create an update, even using bearer tokens for authentication. (secondary throttling was the main issue, according to their own error messages) (In reply to Alexey Dokuchaev from comment #9) > What's the difference between cluster and regular (non-cluster?) versions of these programs? Do we actually need both? More details here: https://docs.victoriametrics.com/cluster-victoriametrics/ But basically the "victoria-metrics" binary is a "all-in-one" single-node application that can scale up to approximately a million datapoint per second, but if you want to have a cluster of server (either because you have higher thruput needs or simply want High Availability even for small usages) you need to use vmselect/vmstorage/vminsert on multiple nodes. Created attachment 247999 [details] victoria-metrics.patch (In reply to Guido Falsi from comment #8) You're right! I've fixed the problem while built with DOCS option. Thx. A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=e44c9efcca8e50a586976ce3b63032ed63208a55 commit e44c9efcca8e50a586976ce3b63032ed63208a55 Author: Alexey Dokuchaev <danfe@FreeBSD.org> AuthorDate: 2024-01-27 10:04:20 +0000 Commit: Alexey Dokuchaev <danfe@FreeBSD.org> CommitDate: 2024-01-27 10:04:20 +0000 net-mgmt/victoria-metrics: update VictoriaMetrics to version 1.93.10 Notably, this fixes the issue of vmagent instances failing to scrape metrics due to frequent crashes, after Golang update to version 1.20 lead to errors (panics) like: panic: BUG: unexpected runtimemetrics.KindBad for sample.Name="/gc/gomemlimit:bytes" PR: 275828, 276639 net-mgmt/victoria-metrics/Makefile | 4 +- net-mgmt/victoria-metrics/Makefile.deps | 65 ++++++++------- net-mgmt/victoria-metrics/distinfo | 136 +++++++++++++++++++------------- 3 files changed, 119 insertions(+), 86 deletions(-) Comment on attachment 247999 [details]
victoria-metrics.patch
Given the size of the patch, it's kind of hard to review and reason about its parts. I'm not asking to split it and/or upload new patches, but intend to consume and commit it piece by piece.
(In reply to Lapo Luchini from comment #11) > [...] if you want to have a cluster of server (either because you have > higher thruput needs or simply want High Availability even for small > usages) you need to use vmselect/vmstorage/vminsert on multiple nodes. Okay, so does it mean that the answer to my question is "no, we don't need two sets of vmutils", i.e. we should just package the cluster one? I'm not sure… it could even be all in a single port (but it would be bulkier on the disk, Go executables tend to be big), or it could make sense to have victoria-metrics-cluster sub-port (and maybe rename the basic one "-single" or similar)? On a single host you either want to run a single "victoria-metrics" (all-in-one) or you are in a cluster and thus want to run (one or all of) "vminsert+vmstorage+vmselect", so the two subports could even exclude one another (even though they are no actual file conflicts). (In reply to Lapo Luchini from comment #16) Looks like the perfect fit for the new SUBPKGS features, but portmgr approval is required for that at present. Curious enough, running Boris patch I'm now getting this error: Performing sanity check on vmagent configuration: 2024/01/27 15:39:51 github.com/VictoriaMetrics/metrics: do not expose go_memlimit_bytes metric, since the corresponding metric /gc/gomemlimit:bytes isn't supported in the current Go runtime /usr/local/etc/rc.d/vmagent: ERROR: FAILED Not sure why, I'm definitely not a go expert. Could be my fault, I wanted to test this patch and the rc script so I kept going even if, in the while, danfe updated the default port. (In reply to Guido Falsi from comment #18) That's actually a warning, but the checkconfig startup script is failing on it. Not sure how checkconfig should treat warnings, but accounting any output as an error looks a little excessive. I usually check the return code. Created attachment 248014 [details] victoria-metrics.patch (In reply to Alexey Dokuchaev from comment #9) > What's the win here for us? I believe GO_MODULE provides more convenient port support (go.mod replace-feauture and other tricks). Based on experience of support www/grafana, ex. > What's the difference between cluster and regular (non-cluster?) versions of these programs? Do we actually need both? Cluster version provides vminsert/vmstorage/vmselect tools. Some peoples need a cluster version. Cluster version SHOULD be a sub-port of net-mgmt/victoria-metrics. (In reply to Alexey Dokuchaev from comment #14) > Given the size of the patch, it's kind of hard to review and reason about its parts. I'm not asking to split it and/or upload new patches, but intend to consume and commit it piece by piece. I've extracted GO_MODULE from the first patch. And suggest it to review. (In reply to Alexey Dokuchaev from comment #15) > Okay, so does it mean that the answer to my question is "no, we don't need two sets of vmutils", i.e. we should just package the cluster one? Regular version of MASTERPORT only is enough for vmutils. (In reply to Lapo Luchini from comment #16) (In reply to Guido Falsi from comment #17) victoria-metrics-cluster in the first version of the patch had used net-mgmt/victoria-metrics as a MASTERPORT. (In reply to Guido Falsi from comment #18) > Curious enough, running Boris patch I'm now getting this error Are you use the last version of the patch? The issue was been fixed at the last version. root@boris:/usr/local/etc/rc.conf.d# service vmagent configtest Performing sanity check on vmagent configuration: 2024/01/27 22:02:31 github.com/VictoriaMetrics/metrics: do not expose go_memlimit_bytes metric, since the corresponding metric /gc/gomemlimit:bytes isn't supported in the current Go runtime OK But there's an issue: https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5670 also. Created attachment 248053 [details]
victoria-metrics-cluster.patch
Full version of the patch
I was trying to upgrade the patch to new LTS version 1.97.2, but that seems to depend on go122 and we need to wait for bug 277091 I guess. Comment on attachment 248014 [details]
victoria-metrics.patch
Maintainer timeout
Comment on attachment 248053 [details]
victoria-metrics-cluster.patch
Maintainer timeout
Now that go1.22 is in the ports I managed to easily update this patch to 1.97.2, but I noticed that BUILDINFO_TAG doesn't seem to work no more, and the binary doesn't include a proper version tag: % vmagent --version vmagent-20240219-145338-08e6100 while current version correctly goes: % vmagent --version vmagent-20240116-231234-tags-v1.93.10-0-gd277977 Created attachment 248654 [details]
Updated patch to latest LTS 1.97.2
Created attachment 248667 [details] victoria-metrics-cluster.patch Updated to 1.73.12 (In reply to Lapo Luchini from comment #25) Yep, you are right. BUILDINFO should contain ${PORTVERSION}. I've fixed the patch. What about update to 1.97.2. There're not a cluster version for 1.97.2. There's v1.97.1-cluster only (has security vulnerability). > There're not a cluster version for 1.97.2.
Unfortunately that seems to have been discontinued for future free LTS releases (like pre-built executables, which we don't care about as the ports compiles it anyways).
Regarding cluster version I guess it's either 1.93.* (supported until dec'24) or using latest non-LTS release, which is always freely available.
BTW: the patch seems to have broken the ability to `service vmagent reload`. I was thinking about the new release rules (and the lack of availability of cluster verison tags for the LTS releases) it could maybe make sense to have both a `victoria-metrics` port and a `victoria-metrics-lts` one and let each user decide? 🤔 Or just use latest instead of LTS. Victoria Metrics 1.100 has nice new features like downsampling. :) https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.100.0 (also 1.100.1 is out already) Created attachment 257102 [details]
victoria-metrics-1.102.12.patch
Switch to GO_MODULE and update to latest LTS 1.102.12 only for easy review.
testport is ok.
|
Created attachment 247142 [details] victoria-metrics.patch The proposed patch switches to GO_MODULE framework. Also added a cluster version of victoria-metrics (with vminsert, vmstorage and vmselect). Also added vmalert rc-script for net-mgmt/vmutils. Sanited rc-scripts.