Skip to content

Add an option to reduce OPA agent memory pressure when loading large data.json files - #366

Draft
Pushpalanka wants to merge 1 commit into
open-policy-agent:mainfrom
Pushpalanka:nested_data_files
Draft

Add an option to reduce OPA agent memory pressure when loading large data.json files#366
Pushpalanka wants to merge 1 commit into
open-policy-agent:mainfrom
Pushpalanka:nested_data_files

Conversation

@Pushpalanka

Copy link
Copy Markdown

Relates: #365

…y pressure

When a system source has multiple HTTP/S3 datasources, OPA's bundle.Write
collapses all data into a single root /data.json in the bundle tar. OPA
agents must parse and hold the entire merged document in memory at once,
causing memory pressure on systems with large datasources.

Add a per-bundle flag options.nested_data_files (default: false). When
enabled, the builder writes each datasource's data as a separate
data.json at its namespaced path within the tar (e.g. /teams/data.json,
/admins/data.json), matching the Styra DAS bundle layout. OPA's bundle
reader merges nested files into the same data document at load time, so
runtime semantics are unchanged.

The flag is per-bundle because OCP builds bundles for many systems with
different data characteristics — systems with many small datasources may
not benefit and would carry extra tar-entry overhead.
Comment thread pkg/builder/builder.go
Comment on lines +511 to 514
if b.nestedDataFiles {
return writeBundle(b.output, *result, bsFSMap)
}
return bundle.Write(b.output, *result)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🏗️ I think this feature would make a lot of sense as an option on bundle. We wouldn't have to replicate the tarball creation logic here, and any OPA users would benefit from it, and also OCP.

What do you think? Would be you OK with pivoting into that direction? 😃

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the direction and taking time to check the draft @srenatus
That makes lot of sense, will try that path. 👍

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! And thanks for contributing, this looks like such a nice tweak. ✨

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants