vmauth crashes - panic: runtime error: invalid memory address or nil pointer dereference #8051
Comments
|
When vminsert was reporting the errors we could see the following logs: |
|
Latest update - deployed v.1.102.0 and we can see the same issues. |
|
Most probably, issue is related to this commit 7ee5797 Introduced at 1.97.7 release. It adds readTrackingBody objects pool and it it looks like, |
|
Hello, could you please provide the following information:
This error should never happen to the vmauth, unless there is unknown bug at An other thing, that could be related to this issue is collocation with vminsert. |
|
Hi @f41gh7,
|
|
Related golang issue: |
|
Minimal reproducible example: func TestMalfunctional(t *testing.T) {
cfgStr := `
unauthorized_user:
url_prefix: {BACKEND}/foo?bar=baz`
requestURL := "http://some-host.com/abc/def?some_arg=some_value"
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
h := w.Header()
h.Set("Connection", "close")
h.Set("Foo", "bar")
var bb bytes.Buffer
if err := r.Header.Write(&bb); err != nil {
panic(fmt.Errorf("unexpected error when marshaling headers: %w", err))
}
fmt.Fprintf(w, "requested_url=http://%s%s\n%s", r.Host, r.URL, bb.String())
}))
defer ts.Close()
cfgStr = strings.ReplaceAll(cfgStr, "{BACKEND}", ts.URL)
if _, err := reloadAuthConfigData([]byte(cfgStr)); err != nil {
t.Fatalf("cannot load config data: %s", err)
}
body := bytes.NewReader([]byte(strings.Repeat("abcd", 1000)))
r, err := http.NewRequest(http.MethodGet, requestURL, body)
if err != nil {
t.Fatalf("cannot initialize http request: %s", err)
}
for range 1000 {
r.RequestURI = r.URL.RequestURI()
r.RemoteAddr = "42.2.3.84:6789"
w := &fakeResponseWriter{}
if !requestHandler(w, r) {
t.Fatalf("unexpected false is returned from requestHandler")
}
body.Seek(0, 0)
}
} |
f41gh7
added a commit
that referenced
this issue
Jan 21, 2025
Sync.Pool for readTrackingBody was added in order to reduce potential load on garbage collector. But golang net/http standard library does not allow to reuse request body, since it closes body asynchronously after return. Related issue: golang/go#51907 This commit removes sync.Pool in order to fix potential panic and data race at requests processing. Related issue: #8051 Signed-off-by: f41gh7 <[email protected]>
|
Hey @hagen1778! I can definitely give it a try with the image you built since it is ready. I hope to have some feedback by tomorrow afternoon - got some important benchmark running until this evening ;-) |
|
Hi @hagen1778. I have good news - I didn't see the issue occurring when I updated vmauth image to the one provided by you yesterday. Run a check against vmauth binary: Attaching two screenshots from the test with fixed vmauth - no errors from vminsert and no pod restarted.
|
hagen1778
added a commit
that referenced
this issue
Jan 22, 2025
hagen1778
added a commit
that referenced
this issue
Jan 22, 2025
Sync.Pool for readTrackingBody was added in order to reduce potential load on garbage collector. But golang net/http standard library does not allow to reuse request body, since it closes body asynchronously after return. Related issue: golang/go#51907 This commit removes sync.Pool in order to fix potential panic and data race at requests processing. Affected releases: - all releases after v1.97.7 Related issue: #8051 Signed-off-by: f41gh7 <[email protected]> Co-authored-by: Roman Khavronenko <[email protected]>
hagen1778
added
the
waiting for release
hagen1778
pushed a commit
that referenced
this issue
Jan 22, 2025
Sync.Pool for readTrackingBody was added in order to reduce potential load on garbage collector. But golang net/http standard library does not allow to reuse request body, since it closes body asynchronously after return. Related issue: golang/go#51907 This commit removes sync.Pool in order to fix potential panic and data race at requests processing. Affected releases: - all releases after v1.97.7 Related issue: #8051 Signed-off-by: f41gh7 <[email protected]> Co-authored-by: Roman Khavronenko <[email protected]> (cherry picked from commit 80ead7c)
|
Hey @hagen1778 , just curious - which release will include the fix? |
It will get into the next 1.110 and 1.102.11. These releases are expected next week. |
f41gh7
removed
the
waiting for release
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
We upgraded vmauth from 1.97.1 to 1.108.1 and when we increased the load on the cluster to 1M datapoints per second and 100+ queries per second we started seeing errors in the Request error rate chart in Victoria Metrics dashboard - see attached screenshot.
After some investigation it turned out that vmauth was crashing with the following stack trace:
We reverted the version to 1.97.1 and the error disappeared.
Later on we started upgrades to 1.101.0 and 1.102.1.
It looks like the bug was introduced in 1.102.1 or between 1.101.0 and 1.102.1.
The trace from 1.102.1 looks as follows:
To Reproduce
Deploy vmauth v1.102.1 which is in front of vminsert and vmselect and run some decent load against the cluster - in our case it was around 1M datapoints per second and 100+ queries per second (vmauth was co-located with vminsert and vmselect as a sidecar and only the one with vminsert crashed) - we used https://github.com/VictoriaMetrics/prometheus-benchmark to generate the load.
Version
Logs
No response
Screenshots
Used command-line flags
No response
Additional information
No response
The text was updated successfully, but these errors were encountered: