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.

Comment 3 Robert Clausecker 2024-01-26 14:47:50 UTC

maintainer timeout

Comment 4 Guido Falsi 2024-01-26 15:32:07 UTC

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?

Comment 6 Guido Falsi 2024-01-26 21:29:08 UTC

(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...

Comment 8 Guido Falsi 2024-01-26 22:12:38 UTC

(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.

Comment 9 Alexey Dokuchaev 2024-01-27 02:16:04 UTC

(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.

Comment 10 Guido Falsi 2024-01-27 08:41:51 UTC

(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.

Comment 13 commit-hook 2024-01-27 10:06:04 UTC

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 14 Alexey Dokuchaev 2024-01-27 10:15:02 UTC

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.

Comment 15 Alexey Dokuchaev 2024-01-27 10:20:54 UTC

(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).

Comment 17 Guido Falsi 2024-01-27 14:04:31 UTC

(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.

Comment 18 Guido Falsi 2024-01-27 14:45:01 UTC

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.

Comment 19 Guido Falsi 2024-01-27 14:56:17 UTC

(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 23 Robert Clausecker 2024-02-20 14:58:52 UTC

Comment on attachment 248014 [details]
victoria-metrics.patch

Maintainer timeout

Comment 24 Robert Clausecker 2024-02-20 14:59:05 UTC

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)