Fix inconsistency in memory variable naming - #415
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR resolves the naming inconsistency for the “reserved memory for ingested data” configuration by standardizing on ingested_* across the server CLI/configuration, Flight protocol, and user-facing docs. It also updates dev documentation to include crate descriptions for previously undocumented crates.
Changes:
- Renamed the reserved-memory setting from
multivariate_*toingested_*across configuration handling, Flight protocol, and server update paths. - Updated integration/unit tests and user documentation to reflect the new setting/flag/environment variable names.
- Added crate descriptions to the developer documentation (and a small formatting-only change in
modelardb_auth).
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| docs/user/README.md | Updates documented CLI flag and env var name for the ingested reserved memory setting. |
| docs/dev/README.md | Adds missing crate descriptions for modelardb_auth and modelardb_macros. |
| crates/modelardb_types/src/flight/protocol.proto | Renames the configuration field and update-setting enum entry to ingested_*. |
| crates/modelardb_server/tests/integration_test.rs | Updates integration tests to use the new proto-generated field/enum names. |
| crates/modelardb_server/src/storage/types.rs | Renames test functions to match the new “ingested” terminology. |
| crates/modelardb_server/src/storage/mod.rs | Wires ingested_reserved_memory_in_bytes() into MemoryPool construction and renames the adjust method. |
| crates/modelardb_server/src/remote/mod.rs | Updates the server-side UpdateConfiguration handling to the new setting name. |
| crates/modelardb_server/src/main.rs | Renames the CLI arg/env var binding to ingested_reserved_memory_in_bytes. |
| crates/modelardb_server/src/configuration.rs | Renames the persisted configuration field and its getters/setters to ingested_*. |
| crates/modelardb_auth/src/lib.rs | Formatting-only change to error mapping closure. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
skejserjensen
approved these changes
Aug 7, 2026
chrthomsen
approved these changes
Aug 7, 2026
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR closes #414 by fixing the inconsistent naming of the memory variable that was either
multivariateoringested. We now useingestedeverywhere. We still usemultivariatewhen talking about the shape of the data but when talking about memory we now useingested.The PR also adds crate descriptions to the dev docs for the two crates that were added earlier without a crate description.