Load packages concurrently in filesystem indexers by mrodm · Pull Request #1489 · elastic/package-registry

mrodm · 2025-11-25T12:38:50Z

This PR updates the file system indexers to load all the packages found in the given paths in parallel using a worker pool.

Doing some local tests:

Before (1m 30secs):

{"log.level":"info","@timestamp":"2025-11-25T12:03:26.751Z","log.origin":{"function":"github.com/elastic/package-registry/packages.(*FileSystemIndexer).getPackagesFromFileSystem","file.name":"packages/packages.go","file.line":247},"message":"Searching packages in /packages/package-storage","service.name":"package-registry","service.version":"1.33.0","ecs.version":"1.6.0"}
{"log.level":"info","@timestamp":"2025-11-25T12:04:59.557Z","log.origin":{"function":"github.com/elastic/package-registry/packages.(*FileSystemIndexer).getPackagesFromFileSystem","file.name":"packages/packages.go","file.line":247},"message":"Searching packages in /packages/package-storage","service.name":"package-registry","service.version":"1.33.0","ecs.version":"1.6.0"}
{"log.level":"info","@timestamp":"2025-11-25T12:04:59.558Z","log.origin":{"function":"main.ensurePackagesAvailable","file.name":"package-registry/main.go","file.line":619},"message":"12427 local package manifests loaded.","service.name":"package-registry","service.version":"1.33.0","ecs.version":"1.6.0"}

After (21secs):

{"log.level":"info","@timestamp":"2025-11-25T12:12:37.866Z","log.origin":{"function":"github.com/elastic/package-registry/packages.(*FileSystemIndexer).getPackagesFromFileSystem","file.name":"packages/packages.go","file.line":263},"message":"Searching packages in filesystem","service.name":"package-registry","service.version":"1.33.1","indexer":"ZipFileSystemIndexer","ecs.version":"1.6.0"}
{"log.level":"info","@timestamp":"2025-11-25T12:12:58.315Z","log.origin":{"function":"github.com/elastic/package-registry/packages.(*FileSystemIndexer).getPackagesFromFileSystem","file.name":"packages/packages.go","file.line":336},"message":"Searching packages in filesystem done","service.name":"package-registry","service.version":"1.33.1","indexer":"ZipFileSystemIndexer","ecs.version":"1.6.0"}
{"log.level":"info","@timestamp":"2025-11-25T12:12:58.369Z","log.origin":{"function":"github.com/elastic/package-registry/packages.(*FileSystemIndexer).getPackagesFromFileSystem","file.name":"packages/packages.go","file.line":263},"message":"Searching packages in filesystem","service.name":"package-registry","service.version":"1.33.1","indexer":"FileSystemIndexer","ecs.version":"1.6.0"}
{"log.level":"info","@timestamp":"2025-11-25T12:12:58.419Z","log.origin":{"function":"github.com/elastic/package-registry/packages.(*FileSystemIndexer).getPackagesFromFileSystem","file.name":"packages/packages.go","file.line":336},"message":"Searching packages in filesystem done","service.name":"package-registry","service.version":"1.33.1","indexer":"FileSystemIndexer","ecs.version":"1.6.0"}
{"log.level":"info","@timestamp":"2025-11-25T12:12:58.420Z","log.origin":{"function":"main.ensurePackagesAvailable","file.name":"package-registry-load-packages-fsys/main.go","file.line":619},"message":"12427 local package manifests loaded.","service.name":"package-registry","service.version":"1.33.1","ecs.version":"1.6.0"}

Author's checklist

Add and run benchmark to load packages in file system indexers.
Add changelog entry
Compare responses returned by the service with the current (main) logic.

How to test this PR locally

# in one terminal
# run the current production image and keep it running to be able to copy the new package-registry binary
docker run --rm --name eprprod -it docker.elastic.co/package-registry/distribution:production

# in other terminal
# build the new package-registry binary with the changes in this PR
# copy that binary into the running container and commit that image to generate a new tag

mage build
docker cp ./package-registry eprprod:/package-registry

docker commit eprprod

# this new docker image is going to run the binary from the EPR
docker tag <sha/id> docker.elastic.co/package-registry/distribution:production-new

# stop the previous container running
docker run --rm -it --name eprnew docker.elastic.co/package-registry/distribution:production-new

prodsecmachine · 2025-11-25T12:39:05Z

✅ Snyk checks have passed. No issues have been found so far.

Status	Scanner	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

jsoriano

Thanks for trying this, this can be a great improvement.

jsoriano · 2025-11-25T17:21:55Z

packages/packages.go

+		position int
+		path     string
+	}
+	numWorkers := runtime.GOMAXPROCS(0) / 2


Did you try directly to use GOMAXPROCS instead of half of them?

I only used half of them because I was being conservative. But, it should not be any problem on setting GOMAXPROCS instead.

jsoriano · 2025-11-25T17:39:38Z

packages/packages.go

+	for range numWorkers {
+		wg.Add(1)
+		go func(logger *zap.Logger, fsBuilder FileSystemBuilder) {
+			defer wg.Done()


Nit, consider moving the workers pool logic to different functions for clarity. Something like this is done in https://github.com/elastic/package-registry/pull/1335/files#diff-8ea6771aea0aa89d25c6b29227dbc22db28f067930f7b9fc9921134c66c744b5R50.

I'll check it.

Updated the code to use the same functions as your link.
I added all these methods as an internal package in 01407b7

teresaromero

this looks like an improvement !

teresaromero · 2025-11-26T09:27:32Z

packages/packages.go

+	}
+	numWorkers := runtime.GOMAXPROCS(0) / 2
+	mu := sync.Mutex{}
 	packagesFound := make(map[packageKey]struct{})


why using here an empty struct? could we use a bool? and chech its found (by key) and true?

Here, I just interested in whether or not the key is present. I think it is not needed to use boolean. An advantage of usingstruct{} here is that its size is also zero according to this.

i need a 💡 for reacting , thanks! learned something today

teresaromero · 2025-11-26T09:36:50Z

packages/packages.go

+			return nil, err
+		}
+		for _, path := range packagePaths {
+			jobChan <- packageJob{position: count, path: path}


do we need the paths to keep the original positions? 🤔

I thought to keep the same ordering that it was now, to ensure there are no changes in the list generated.

mrodm · 2025-11-26T16:05:51Z

Some benchmarks run with a small subset of packages from testdata folder using the benchmark added in c9eff98:

main vs loading in parallel using runtime.GOMAXPROCS(0)/2:

goos: linux
goarch: amd64
pkg: github.com/elastic/package-registry/packages
cpu: 11th Gen Intel(R) Core(TM) i7-11800H @ 2.30GHz
        │ sequential_load_packages.txt │ parallel_load_packages_half_cpus.txt │
        │            sec/op            │    sec/op     vs base                │
Init-16                    37.12m ± 7%    15.57m ± 3%  -58.06% (p=0.000 n=10)

        │ sequential_load_packages.txt │ parallel_load_packages_half_cpus.txt  │
        │             B/op             │     B/op       vs base                │
Init-16                   9.647Mi ± 0%   11.998Mi ± 0%  +24.37% (p=0.000 n=10)

        │ sequential_load_packages.txt │ parallel_load_packages_half_cpus.txt │
        │          allocs/op           │   allocs/op    vs base               │
Init-16                    197.6k ± 0%     199.2k ± 0%  +0.84% (p=0.000 n=10)

main vs loading in parallel using runtime.GOMAXPROCS(0):

goos: linux
goarch: amd64
pkg: github.com/elastic/package-registry/packages
cpu: 11th Gen Intel(R) Core(TM) i7-11800H @ 2.30GHz
        │ sequential_load_packages.txt │ parallel_load_packages_all_cpus.txt  │
        │            sec/op            │    sec/op     vs base                │
Init-16                    37.12m ± 7%   18.29m ± 40%  -50.72% (p=0.000 n=10)

        │ sequential_load_packages.txt │  parallel_load_packages_all_cpus.txt  │
        │             B/op             │     B/op       vs base                │
Init-16                   9.647Mi ± 0%   12.217Mi ± 1%  +26.65% (p=0.000 n=10)

        │ sequential_load_packages.txt │ parallel_load_packages_all_cpus.txt │
        │          allocs/op           │  allocs/op    vs base               │
Init-16                    197.6k ± 0%    199.4k ± 0%  +0.90% (p=0.000 n=10)

This shows that there is an improvement in speed (secs/op), but as it must read more packages in parallel, there is also an increase in memory related to the number of CPUs used.

mrodm · 2025-11-26T17:13:17Z

Looking at this error: https://buildkite.com/elastic/package-registry/builds/1528#019ac11a-cfdc-4fb0-9eeb-024454995a6b/118-1448

It happens some time when running go test -count=1 ./... -v

mrodm · 2025-11-26T18:02:14Z

After 9241261 , I re-run the benchmark and this is the result using all the CPUs available:

goos: linux
goarch: amd64
pkg: github.com/elastic/package-registry/packages
cpu: 11th Gen Intel(R) Core(TM) i7-11800H @ 2.30GHz
        │ sequential_load_packages.txt │ parallel_load_packages_all_cpus_duplicates.txt │
        │            sec/op            │         sec/op          vs base                │
Init-16                    37.12m ± 7%              16.05m ± 1%  -56.77% (p=0.000 n=10)

        │ sequential_load_packages.txt │ parallel_load_packages_all_cpus_duplicates.txt │
        │             B/op             │          B/op           vs base                │
Init-16                   9.647Mi ± 0%            12.177Mi ± 0%  +26.23% (p=0.000 n=10)

        │ sequential_load_packages.txt │ parallel_load_packages_all_cpus_duplicates.txt │
        │          allocs/op           │        allocs/op         vs base               │
Init-16                    197.6k ± 0%               199.3k ± 0%  +0.89% (p=0.000 n=10)

It follows the same pattern as in #1489 (comment)

CHANGELOG.md

mrodm · 2025-11-27T12:46:57Z

packages/packages.go

-			if _, found := packagesFound[key]; found {
-				i.logger.Debug("duplicated package",
-					zap.String("package.name", p.Name),
-					zap.String("package.version", p.Version),
-					zap.String("package.path", p.BasePath))
-				continue
-			}


This process is now moved out of this loop to ensure the same ordering as before is kept.
Considering that the same package is defined in different paths, performing the load of package concurrently could lead to load first into the array the package from the second path but not the package from the first path.

The current implementation loads all the packages in the expected position (as it was before). And then once finished this loop, duplicated packages are removed following the order (keeping the first packages defined).

mrodm · 2025-11-27T13:15:10Z

packages/packages.go

-			if err != nil {
-				return nil, fmt.Errorf("loading package failed (path: %s): %w", path, err)
-			}
+	taskPool := workers.NewTaskPool(runtime.GOMAXPROCS(0))


Currently, the number of workers is hardcoded to be GOMAXPROCS number.

Should it be added some flag to be able to override this setting ? Maybe we could keep it as is for now. WDYT?

from what i've read, this default has been "checked" at 1.25 making it more flexible, so it will take whatever is able to take, even in containers (previous 1.25 it took the machine limit, now it takes the cpu limit of the container) https://go.dev/blog/container-aware-gomaxprocs

given this, i would leave it as hardcoded GOMAXPROCS to have the max possible. perhaps we could monitor and change this in the future if it takes too many resources.

although registry is still at 1.24 so this number is taking the cpu available on the machine when running inside a container 🤔 so until we upgrade maybe we have to limit as an option :D

Thanks!
Binaries are built with the version set in .go-version and currently it is 1.25.3.

package-registry/.go-version

Line 1 in e42e823

1.25.3

However our go.mod is still pointing to 1.24.

package-registry/go.mod

Line 3 in e42e823

go 1.24.0

Just in case, I've added a new flag to be able to override this value.
By default, it will be runtime.GOMAXPROCS(0).

packages/packages.go

jsoriano · 2025-11-27T17:34:37Z

packages/packages.go

+		return nil, err
+	}
+
+	// Remove duplicates while preserving filesystem discovery order.


Out of curiosity, is it needed to preserve filesystem order? It could be quite random.

This could be re-phrased. At least, it needs to be kept the same ordering of appearance of packages depending on which path they appear.
Those packages defined in the first path set by the user have precedence if they also are defined in the following paths defined by the user.

There is also a test that checks that too:

package-registry/testdata/generated/package/multiversion/1.1.0/index.json

Line 3 in e42e823

"title": "Multi Version Second with the same version! This one should win, because it is first.",

From this test:

package-registry/main_test.go

Lines 535 to 541 in e42e823

for _, test := range tests {

t.Run(test.PackageName+"/"+test.PackageVersion, func(t *testing.T) {

packageEndpoint := "/package/" + test.PackageName + "/" + test.PackageVersion + "/"

fileName := filepath.Join("package", test.PackageName, test.PackageVersion, "index.json")

runEndpoint(t, packageEndpoint, packageIndexRouterPath, fileName, packageIndexHandler)

})

}

Updated comment here e9ca9a6

mrodm · 2025-11-27T19:22:26Z

main.go

 	flag.StringVar(&proxyTo, "proxy-to", "https://epr.elastic.co/", "Proxy-to endpoint")

 	flag.BoolVar(&packagePathsEnableWatcher, "package-paths-enable-watcher", false, "Enable file system watcher for package paths to automatically detect new packages.")
+	flag.IntVar(&packagePathsWorkers, "package-paths-workers", runtime.GOMAXPROCS(0), "Number of workers to use for reading packages concurrently from the configured paths. Default is the number of CPU cores returned by GOMAXPROCS.")


Tested using flag and environment variable

./package-registry -log-level debug -package-paths-workers=5 EPR_PACKAGE_PATHS_WORKERS=2 ./package-registry -log-level debug

…urrently

mrodm · 2025-11-27T19:32:26Z

main.go

+	logger.Debug("Using workers to read packages from package paths", zap.Int("workers", fsOptions.PathsWorkers))
+	logger.Debug("Watching package paths for changes", zap.Bool("enabled", fsOptions.EnablePathsWatcher))


I thought to show in debug level the values used by the service. WDYT ?

Examples of logs shown:

{ "log.level": "debug", "@timestamp": "2025-11-27T20:17:56.815+0100", "log.origin": { "function": "main.initIndexer", "file.name": "package-registry-load-packages-fsys/main.go", "file.line": 407 }, "message": "Using workers to read packages from package paths", "service.name": "package-registry", "service.version": "1.33.1", "workers": 16, "ecs.version": "1.6.0" } { "log.level": "debug", "@timestamp": "2025-11-27T20:17:56.815+0100", "log.origin": { "function": "main.initIndexer", "file.name": "package-registry-load-packages-fsys/main.go", "file.line": 408 }, "message": "Wathching package paths for changes", "service.name": "package-registry", "service.version": "1.33.1", "enabled": false, "ecs.version": "1.6.0" }

elasticmachine · 2025-11-27T20:23:53Z

💚 Build Succeeded

Buildkite Build
Commit: fcb290e

History

💚 Build #1537 succeeded e9ca9a6
💚 Build #1536 succeeded 6dcc859
💚 Build #1534 succeeded 21ec258
💚 Build #1531 succeeded 4e78562
💚 Build #1529 succeeded 9e0a0d0
💔 Build #1528 failed 024d5d3

cc @mrodm

Load in parallel packages from filesystem

6ed5f33

mrodm self-assigned this Nov 25, 2025

jsoriano reviewed Nov 25, 2025

View reviewed changes

teresaromero reviewed Nov 26, 2025

View reviewed changes

mrodm added 2 commits November 26, 2025 17:06

Add benchmark

c9eff98

Refactor workers logic into functions - moved to an internal package

01407b7

mrodm force-pushed the concurrent-load-packages-fsys branch from 5128360 to 01407b7 Compare November 26, 2025 16:23

mrodm added 2 commits November 26, 2025 17:58

Add changelog entries

024d5d3

Rephrase changelog entry

9e0a0d0

Ensure packages kept in the final list are the ones loaded first

9241261

mrodm added 4 commits November 26, 2025 19:03

Re-phrase comment

4e78562

Merge upstream/main into concurrent-load-packages-fsys

6612363

Fix parameter for filesystem constructors

21ec258

Update changelog entry descriptions

44f3419

mrodm commented Nov 27, 2025

View reviewed changes

CHANGELOG.md Show resolved Hide resolved

Assign directly GOMAXPROCS as parameter

6dcc859

mrodm commented Nov 27, 2025

View reviewed changes

mrodm changed the title ~~[PoC] Load in parallel packages from filesystem~~ Load packages concurrently in filesystem indexers Nov 27, 2025

mrodm marked this pull request as ready for review November 27, 2025 13:15

mrodm requested a review from a team as a code owner November 27, 2025 13:15

mrodm requested review from jsoriano and teresaromero November 27, 2025 13:15

jsoriano approved these changes Nov 27, 2025

View reviewed changes

mrodm added 2 commits November 27, 2025 19:34

Add changelog entry

5aea1c6

Remove unnecessary function

a62499d

Rephrase comment about duplicates

e9ca9a6

mrodm commented Nov 27, 2025

View reviewed changes

mrodm requested a review from jsoriano November 27, 2025 19:26

Add flag to override number of workers assigned to read packages conc…

fcb290e

…urrently

mrodm force-pushed the concurrent-load-packages-fsys branch from d182ee8 to fcb290e Compare November 27, 2025 19:32

mrodm commented Nov 27, 2025

View reviewed changes

jsoriano approved these changes Nov 27, 2025

View reviewed changes

teresaromero approved these changes Nov 28, 2025

View reviewed changes

mrodm merged commit d659d7f into elastic:main Nov 28, 2025
5 checks passed

	for _, test := range tests {
	t.Run(test.PackageName+"/"+test.PackageVersion, func(t *testing.T) {
	packageEndpoint := "/package/" + test.PackageName + "/" + test.PackageVersion + "/"
	fileName := filepath.Join("package", test.PackageName, test.PackageVersion, "index.json")
	runEndpoint(t, packageEndpoint, packageIndexRouterPath, fileName, packageIndexHandler)
	})
	}

		logger.Debug("Using workers to read packages from package paths", zap.Int("workers", fsOptions.PathsWorkers))
		logger.Debug("Watching package paths for changes", zap.Bool("enabled", fsOptions.EnablePathsWatcher))

Conversation

mrodm commented Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Author's checklist

How to test this PR locally

Uh oh!

prodsecmachine commented Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Snyk checks have passed. No issues have been found so far.

Uh oh!

jsoriano left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teresaromero left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrodm commented Nov 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mrodm commented Nov 26, 2025

Uh oh!

mrodm commented Nov 26, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Nov 27, 2025

💚 Build Succeeded

History

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mrodm commented Nov 25, 2025 •

edited

Loading

prodsecmachine commented Nov 25, 2025 •

edited

Loading

mrodm commented Nov 26, 2025 •

edited

Loading