From 36f974a2a7c8815058ab561f27603c4ead6beb15 Mon Sep 17 00:00:00 2001 From: Keegan Carruthers-Smith Date: Sun, 13 Apr 2025 09:58:04 +0200 Subject: [PATCH] all: re-implement Windows support This commit re-adds support for building and running Zoekt on Windows platforms. We had previously reverted Windows support in commits 70647bad55 and 0d03621d45 due to possible memory mapping issues affecting Linux servers. The key changes are: - Added platform-specific implementations for file handling - Re-introduced Windows-specific signal handling - Fixed disk usage reporting on Windows via filepath.VolumeName - Used OS-specific build tags to keep the page size hardcoded on non-Windows platforms. We maintain the approach of using a hardcoded page size (4KB) on non-Windows platforms, which was the key reason for the initial support removal, while providing correct Windows implementations. Test Plan: Successfully builds with GOOS=windows GOARCH=amd64 --- cmd/zoekt-webserver/main.go | 28 ++++++-- cmd/zoekt-webserver/main_unix.go | 9 +++ cmd/zoekt-webserver/main_windows.go | 23 +++++++ cmd/zoekt-webserver/metrics.go | 85 ------------------------- cmd/zoekt-webserver/metrics_nonlinux.go | 17 +++++ index/builder.go | 6 -- index/builder_unix.go | 14 ++++ index/builder_windows.go | 22 +++++++ index/indexfile.go | 2 +- index/indexfile_windows.go | 60 +++++++++++++++++ index/tombstones_windows.go | 20 ++++++ 11 files changed, 189 insertions(+), 97 deletions(-) create mode 100644 cmd/zoekt-webserver/main_unix.go create mode 100644 cmd/zoekt-webserver/main_windows.go delete mode 100644 cmd/zoekt-webserver/metrics.go create mode 100644 cmd/zoekt-webserver/metrics_nonlinux.go create mode 100644 index/builder_unix.go create mode 100644 index/builder_windows.go create mode 100644 index/indexfile_windows.go create mode 100644 index/tombstones_windows.go diff --git a/cmd/zoekt-webserver/main.go b/cmd/zoekt-webserver/main.go index f3218be1f..3c2286b64 100644 --- a/cmd/zoekt-webserver/main.go +++ b/cmd/zoekt-webserver/main.go @@ -32,6 +32,7 @@ import ( "os" "os/signal" "path/filepath" + "runtime" "strconv" "strings" "time" @@ -45,7 +46,6 @@ import ( "github.com/uber/jaeger-client-go" oteltrace "go.opentelemetry.io/otel/trace" "go.uber.org/automaxprocs/maxprocs" - "golang.org/x/sys/unix" "google.golang.org/grpc" "github.com/sourcegraph/zoekt" @@ -349,8 +349,8 @@ func addProxyHandler(mux *http.ServeMux, socket string) { // times you will read the channel (used as buffer for signal.Notify). func shutdownSignalChan(maxReads int) <-chan os.Signal { c := make(chan os.Signal, maxReads) - signal.Notify(c, os.Interrupt) // terminal C-c and goreman - signal.Notify(c, unix.SIGTERM) // Kubernetes + signal.Notify(c, os.Interrupt) // terminal C-c and goreman + signal.Notify(c, PLATFORM_SIGTERM) // Kubernetes return c } @@ -459,13 +459,28 @@ Possible remediations: } } +func diskUsage(path string) (*disk.UsageStat, error) { + duPath := path + if runtime.GOOS == "windows" { + duPath = filepath.VolumeName(duPath) + } + usage, err := disk.Usage(duPath) + if err != nil { + return nil, fmt.Errorf("diskUsage: %w", err) + } + return usage, err +} + func mustRegisterDiskMonitor(path string) { prometheus.MustRegister(prometheus.NewGaugeFunc(prometheus.GaugeOpts{ Name: "src_disk_space_available_bytes", Help: "Amount of free space disk space.", ConstLabels: prometheus.Labels{"path": path}, }, func() float64 { - usage, _ := disk.Usage(path) + // I know there is no error handling here, and I don't like it + // but there was no error handling in the previous version + // that used Statfs, either, so I'm assuming there's no need for it + usage, _ := diskUsage(path) return float64(usage.Free) })) @@ -474,7 +489,10 @@ func mustRegisterDiskMonitor(path string) { Help: "Amount of total disk space.", ConstLabels: prometheus.Labels{"path": path}, }, func() float64 { - usage, _ := disk.Usage(path) + // I know there is no error handling here, and I don't like it + // but there was no error handling in the previous version + // that used Statfs, either, so I'm assuming there's no need for it + usage, _ := diskUsage(path) return float64(usage.Total) })) } diff --git a/cmd/zoekt-webserver/main_unix.go b/cmd/zoekt-webserver/main_unix.go new file mode 100644 index 000000000..cdcc52a0d --- /dev/null +++ b/cmd/zoekt-webserver/main_unix.go @@ -0,0 +1,9 @@ +//go:build !windows + +package main + +import ( + platform "golang.org/x/sys/unix" +) + +const PLATFORM_SIGTERM = platform.SIGTERM diff --git a/cmd/zoekt-webserver/main_windows.go b/cmd/zoekt-webserver/main_windows.go new file mode 100644 index 000000000..0227ee795 --- /dev/null +++ b/cmd/zoekt-webserver/main_windows.go @@ -0,0 +1,23 @@ +// Copyright 2016 Google Inc. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + platform "golang.org/x/sys/windows" +) + +const PLATFORM_SIGTERM = platform.SIGTERM + +// Memory map metrics are provided in metrics_nonlinux.go diff --git a/cmd/zoekt-webserver/metrics.go b/cmd/zoekt-webserver/metrics.go deleted file mode 100644 index 160d5ddf9..000000000 --- a/cmd/zoekt-webserver/metrics.go +++ /dev/null @@ -1,85 +0,0 @@ -package main - -import ( - "path" - - "github.com/prometheus/client_golang/prometheus" - "github.com/prometheus/procfs" - sglog "github.com/sourcegraph/log" -) - -func mustRegisterMemoryMapMetrics(logger sglog.Logger) { - logger = logger.Scoped("memoryMapMetrics") - - // The memory map metrics are collected via /proc, which - // is only available on linux-based operating systems. - - // Instantiate shared FS objects for accessing /proc and /proc/self, - // and skip metrics registration if we're aren't able to instantiate them - // for whatever reason. - - fs, err := procfs.NewDefaultFS() - if err != nil { - logger.Debug( - "skipping registration", - sglog.String("reason", "failed to initialize proc FS"), - sglog.String("error", err.Error()), - ) - - return - } - - info, err := fs.Self() - if err != nil { - logger.Debug( - "skipping registration", - sglog.String("path", path.Join(procfs.DefaultMountPoint, "self")), - sglog.String("reason", "failed to initialize process info object for current process"), - sglog.String("error", err.Error()), - ) - - return - } - - // Register Prometheus memory map metrics - - prometheus.MustRegister(prometheus.NewGaugeFunc(prometheus.GaugeOpts{ - Name: "proc_metrics_memory_map_max_limit", - Help: "Upper limit on amount of memory mapped regions a process may have.", - }, func() float64 { - vm, err := fs.VM() - if err != nil { - logger.Debug( - "failed to read virtual memory statistics for the current process", - sglog.String("path", path.Join(procfs.DefaultMountPoint, "sys", "vm")), - sglog.String("error", err.Error()), - ) - - return 0 - } - - if vm.MaxMapCount == nil { - return 0 - } - - return float64(*vm.MaxMapCount) - })) - - prometheus.MustRegister(prometheus.NewGaugeFunc(prometheus.GaugeOpts{ - Name: "proc_metrics_memory_map_current_count", - Help: "Amount of memory mapped regions this process is currently using.", - }, func() float64 { - procMaps, err := info.ProcMaps() - if err != nil { - logger.Debug( - "failed to read memory mappings for current process", - sglog.String("path", path.Join(procfs.DefaultMountPoint, "self", "maps")), - sglog.String("error", err.Error()), - ) - - return 0 - } - - return float64(len(procMaps)) - })) -} diff --git a/cmd/zoekt-webserver/metrics_nonlinux.go b/cmd/zoekt-webserver/metrics_nonlinux.go new file mode 100644 index 000000000..5412363ed --- /dev/null +++ b/cmd/zoekt-webserver/metrics_nonlinux.go @@ -0,0 +1,17 @@ +//go:build !linux + +package main + +import ( + sglog "github.com/sourcegraph/log" +) + +func mustRegisterMemoryMapMetrics(logger sglog.Logger) { + // The memory map metrics are collected via /proc, which + // is only available on linux-based operating systems. + + // Other platforms do not have the same virtual memory statistics as Linux. + // For example, Windows does not have the concept of + // a count of memory maps, and a max number of memory maps + return +} diff --git a/index/builder.go b/index/builder.go index 46f342d59..58c1c32f7 100644 --- a/index/builder.go +++ b/index/builder.go @@ -40,7 +40,6 @@ import ( "github.com/dustin/go-humanize" "github.com/go-enry/go-enry/v2" "github.com/rs/xid" - "golang.org/x/sys/unix" "maps" @@ -1076,8 +1075,3 @@ func (e *deltaIndexOptionsMismatchError) Error() string { // umask holds the Umask of the current process var umask os.FileMode - -func init() { - umask = os.FileMode(unix.Umask(0)) - unix.Umask(int(umask)) -} diff --git a/index/builder_unix.go b/index/builder_unix.go new file mode 100644 index 000000000..45f772fdc --- /dev/null +++ b/index/builder_unix.go @@ -0,0 +1,14 @@ +//go:build !windows + +package index + +import ( + "os" + + "golang.org/x/sys/unix" +) + +func init() { + umask = os.FileMode(unix.Umask(0)) + unix.Umask(int(umask)) +} diff --git a/index/builder_windows.go b/index/builder_windows.go new file mode 100644 index 000000000..38ac9a58c --- /dev/null +++ b/index/builder_windows.go @@ -0,0 +1,22 @@ +// Copyright 2018 Google Inc. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package index + +// No OS-specific imports needed + +func init() { + // no setting of file permissions on Windows + umask = 0 +} diff --git a/index/indexfile.go b/index/indexfile.go index e9ddb3b5d..daa8e6606 100644 --- a/index/indexfile.go +++ b/index/indexfile.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -//go:build linux || darwin +//go:build !windows package index diff --git a/index/indexfile_windows.go b/index/indexfile_windows.go new file mode 100644 index 000000000..2ffed7b58 --- /dev/null +++ b/index/indexfile_windows.go @@ -0,0 +1,60 @@ +// Copyright 2016 Google Inc. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package index + +import ( + "fmt" + "math" + "os" +) + +// NewIndexFile returns a new index file. The index file takes +// ownership of the passed in file, and may close it. +func NewIndexFile(f *os.File) (IndexFile, error) { + return &indexFileFromOS{f}, nil +} + +type indexFileFromOS struct { + f *os.File +} + +func (f *indexFileFromOS) Read(off, sz uint32) ([]byte, error) { + r := make([]byte, sz) + _, err := f.f.ReadAt(r, int64(off)) + return r, err +} + +func (f indexFileFromOS) Size() (uint32, error) { + fi, err := f.f.Stat() + if err != nil { + return 0, err + } + + sz := fi.Size() + + if sz >= math.MaxUint32 { + return 0, fmt.Errorf("overflow") + } + + return uint32(sz), nil +} + +func (f indexFileFromOS) Close() { + f.f.Close() +} + +func (f indexFileFromOS) Name() string { + return f.f.Name() +} diff --git a/index/tombstones_windows.go b/index/tombstones_windows.go new file mode 100644 index 000000000..56b4b92e4 --- /dev/null +++ b/index/tombstones_windows.go @@ -0,0 +1,20 @@ +// Copyright 2018 Google Inc. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package index + +func init() { + // no setting of file permissions on Windows + umask = 0 +}