-
Notifications
You must be signed in to change notification settings - Fork 6
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
internal/counter: fix windows mapped file extension
On Windows, unmapping the previous mappedFile in mmap.Mmap caused panics when counter pointers were read concurrent to the remapping. Upon investigation, it appears that the unmapping was only necessary for tests, to ensure that previous mappings were cleaned up (and therefore that test files can be deleted). A call to runtime.SetFinalizer(..., mappedFile.close) appeared to serve a similar purpose, yet for an unknown reason finalizers were never run. Deeper investigation revealed that there was simply one bug in file cleanup (coincidentally already noted in a TODO): after storing the newly mapped file in file.newCounter1 and invalidating counters, we can close the previous mappedFile. Therefore: - fix the cleanup in file.newCounter1 - remove the unmap in mmap.Mmap on windows - remove the now unnecessary 'existing' parameter in mmap APIs - remove the SetFinalizer call - add a test for multiple concurrent mappings of a file - add an end-to-end test for concurrent file extension - change ReadCounter to read by memory mapping the file, in an attempt to avoid msync issues For golang/go#68311 Fixes golang/go#68358 Change-Id: I27b6f4f4939e93f7c76f920d553848bf014be236 Reviewed-on: https://go-review.googlesource.com/c/telemetry/+/597278 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Hyang-Ah Hana Kim <[email protected]>
- Loading branch information
Showing
10 changed files
with
281 additions
and
65 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| // Copyright 2024 The Go Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style | ||
| // license that can be found in the LICENSE file. | ||
|
|
||
| package counter_test | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "os" | ||
| "path/filepath" | ||
| "strings" | ||
| "sync" | ||
| "testing" | ||
| "time" | ||
|
|
||
| "golang.org/x/telemetry/counter/countertest" | ||
| "golang.org/x/telemetry/internal/counter" | ||
| "golang.org/x/telemetry/internal/regtest" | ||
| "golang.org/x/telemetry/internal/telemetry" | ||
| "golang.org/x/telemetry/internal/testenv" | ||
| ) | ||
|
|
||
| func TestConcurrentExtension(t *testing.T) { | ||
| testenv.SkipIfUnsupportedPlatform(t) | ||
|
|
||
| // This test verifies that files may be concurrently extended: when one file | ||
| // discovers that its entries exceed the mapped data, it remaps the data. | ||
|
|
||
| // Both programs populate enough new records to extend the file multiple | ||
| // times. | ||
| const numCounters = 50000 | ||
| prog1 := regtest.NewProgram(t, "inc1", func() int { | ||
| for i := 0; i < numCounters; i++ { | ||
| counter.New(fmt.Sprint("gophers", i)).Inc() | ||
| } | ||
| return 0 | ||
| }) | ||
| prog2 := regtest.NewProgram(t, "inc2", func() int { | ||
| for i := numCounters; i < 2*numCounters; i++ { | ||
| counter.New(fmt.Sprint("gophers", i)).Inc() | ||
| } | ||
| return 0 | ||
| }) | ||
|
|
||
| dir := t.TempDir() | ||
| now := time.Now().UTC() | ||
|
|
||
| // Run a no-op program in the telemetry dir to ensure that the weekends file | ||
| // exists, and avoid the race described in golang/go#68390. | ||
| // (We could also call countertest.Open here, but better to avoid mutating | ||
| // state in the current process for a test that is otherwise hermetic) | ||
| prog0 := regtest.NewProgram(t, "init", func() int { return 0 }) | ||
| if _, err := regtest.RunProgAsOf(t, dir, now, prog0); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
|
|
||
| var wg sync.WaitGroup | ||
| wg.Add(2) | ||
|
|
||
| // Run the programs concurrently. | ||
| go func() { | ||
| defer wg.Done() | ||
| if out, err := regtest.RunProgAsOf(t, dir, now, prog1); err != nil { | ||
| t.Errorf("prog1 failed: %v; output:\n%s", err, out) | ||
| } | ||
| }() | ||
| go func() { | ||
| defer wg.Done() | ||
| if out, err := regtest.RunProgAsOf(t, dir, now, prog2); err != nil { | ||
| t.Errorf("prog2 failed: %v; output:\n%s", err, out) | ||
| } | ||
| }() | ||
|
|
||
| wg.Wait() | ||
|
|
||
| counts := readCountsForDir(t, telemetry.NewDir(dir).LocalDir()) | ||
| if got, want := len(counts), 2*numCounters; got != want { | ||
| t.Errorf("Got %d counters, want %d", got, want) | ||
| } | ||
|
|
||
| for name, value := range counts { | ||
| if value != 1 { | ||
| t.Errorf("count(%s) = %d, want 1", name, value) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func readCountsForDir(t *testing.T, dir string) map[string]uint64 { | ||
| entries, err := os.ReadDir(dir) | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| var countFiles []string | ||
| for _, entry := range entries { | ||
| if strings.HasSuffix(entry.Name(), ".count") { | ||
| countFiles = append(countFiles, filepath.Join(dir, entry.Name())) | ||
| } | ||
| } | ||
| if len(countFiles) != 1 { | ||
| t.Fatalf("found %d count files, want 1; directory contents: %v", len(countFiles), entries) | ||
| } | ||
|
|
||
| counters, _, err := countertest.ReadFile(countFiles[0]) | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| return counters | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.