Skip to content

chore(core): add preferences to /settings #5621

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 70 commits into from
May 22, 2025
Merged

Conversation

glasstiger
Copy link
Contributor

@glasstiger glasstiger commented Apr 24, 2025

Functionality to save key-value pairs (currently referred to as preferences) on the server into a file via a POST or PUT request sent to the /settings endpoint.
The saved preferences returned back to the client in the response of a GET request sent to the /settings endpoint.

  • PUT: overwrite mode, current preferences gets replaced with the one sent in the request
  • POST: merge mode, current preferences gets merged with the one sent in the request
  • GET: returns settings which consists of 2 parts, config which comes from server.conf and preferences

Preferences are sent/received in json format, and versioned.
When preferences are updated, the version on which the changes are based has to be sent to the server.
If the base version is not matching with the current version, meaning another update was already processed, the update request is rejected.
The client has to query the latest version of preferences, and re-send any changes it wants to make.

The web console will use this to save instance name, description and colour as the first step, and the server will take these from preferences instead of env variables.
Later we can move reloadable configuration or other environment variables into preferences, and build a screen in the web console to manage them.


This PR also contains some refactorings:

  • HTTP request validation: now processors declare what kind of requests they can accept
  • HTTP processor lookup: there is a bit more abstraction now, instead of registering processors directly for each URI, we register request handlers now, and the request handlers provide the right processor for each request based on their HTTP header.

Example API calls:

  • query preferences:
/settings
GET
Response: {
  "config":{...},
  "preferences.version":0,
  "preferences":{}
}
  • overwrite current preferences:
/settings?version=0
PUT
{"key1":"value1","key2":"value2"}
Response: OK

/settings
GET
Response: {
  "config":{...},
  "preferences.version":1,
  "preferences":{"key1":"value1","key2":"value2"}
}
  • merge changes with current preferences:
/settings?version=1
POST
{"key3":"value3","key2":"value222"}
Response: OK

/settings
GET
Response: {
  "config":{...},
  "preferences.version":2,
  "preferences":{"key1":"value1","key2":"value222","key3":"value3"}
}

Required by:
https://github.com/questdb/questdb-enterprise/pull/614
questdb/ui#427


TODO:

  • UX, Web Console changes - @emrberk
  • test improvements: fragmentation test, error scenarios - @glasstiger
  • move hardcoded initial pool size into config - @glasstiger
  • return the saved config via the /settings endpoint - @glasstiger
  • add versioning, reject update if not based on the current version, web console can re-try - @glasstiger
  • accept a URL param called mode which can have two values: overwrite and merge. overwrite replaces the config with the one sent, merge is treated as an incremental update with dedup (new value wins) - @glasstiger
  • add support for non-multipart POST - @glasstiger
  • create json parser for the config, and save key-value pairs into the file instead of full json string - @glasstiger
  • concurrent tests - @glasstiger

@glasstiger glasstiger marked this pull request as ready for review May 6, 2025 19:36
@bluestreak01
Copy link
Member

@glasstiger this is a very good PR, fragmentation works, optimistic locking works, all good! One small thing, which could be a "nit":

The error response from PUT call is text, e.g. when version is mismatched. Perhaps we should make this response JSON too for the Web Console? To make it easier have intelligent response to server error?

@bluestreak01
Copy link
Member

in fact Web Console does expect JSON from 400 already. It just errors out in the console right now.

@glasstiger
Copy link
Contributor Author

@glasstiger this is a very good PR, fragmentation works, optimistic locking works, all good! One small thing, which could be a "nit":

The error response from PUT call is text, e.g. when version is mismatched. Perhaps we should make this response JSON too for the Web Console? To make it easier have intelligent response to server error?

Changed it in 341eede to return HTTP 409 (Conflict) instead of HTTP 400 (Bad Request).
The web console can detect version conflict by checking for 409 status code.

@glasstiger
Copy link
Contributor Author

[PR Coverage check]

😍 pass : 491 / 511 (96.09%)

file detail

path covered line new line coverage
🔵 io/questdb/cutlass/http/HttpRequestHandler.java 0 1 00.00%
🔵 io/questdb/cutlass/http/processors/TextImportProcessor.java 1 2 50.00%
🔵 io/questdb/cutlass/http/processors/StaticContentProcessor.java 1 2 50.00%
🔵 io/questdb/std/str/CharSink.java 9 15 60.00%
🔵 io/questdb/cutlass/http/processors/RejectProcessor.java 2 3 66.67%
🔵 io/questdb/cutlass/http/HttpResponseSink.java 10 11 90.91%
🔵 io/questdb/std/str/Utf16Sink.java 10 11 90.91%
🔵 io/questdb/cutlass/json/AbstractJsonParser.java 44 47 93.62%
🔵 io/questdb/cutlass/http/HttpRequestValidator.java 36 38 94.74%
🔵 io/questdb/preferences/SettingsStore.java 72 74 97.30%
🔵 io/questdb/preferences/PreferencesMap.java 60 61 98.36%
🔵 io/questdb/cutlass/http/processors/SettingsProcessor.java 71 71 100.00%
🔵 io/questdb/PropServerConfiguration.java 17 17 100.00%
🔵 io/questdb/cutlass/http/HttpHeaderParser.java 9 9 100.00%
🔵 io/questdb/cutlass/http/HttpRequestProcessor.java 1 1 100.00%
🔵 io/questdb/ServerConfiguration.java 7 7 100.00%
🔵 io/questdb/PublicPassthroughConfiguration.java 1 1 100.00%
🔵 io/questdb/cutlass/http/processors/HealthCheckProcessor.java 2 2 100.00%
🔵 io/questdb/cutlass/http/HttpConnectionContext.java 29 29 100.00%
🔵 io/questdb/cutlass/http/HttpMultipartContentProcessor.java 1 1 100.00%
🔵 io/questdb/cutlass/http/processors/LineHttpPingProcessor.java 3 3 100.00%
🔵 io/questdb/preferences/PreferencesParser.java 18 18 100.00%
🔵 io/questdb/cutlass/json/JsonLexer.java 5 5 100.00%
🔵 io/questdb/cutlass/http/processors/JsonQueryProcessor.java 1 1 100.00%
🔵 io/questdb/cutlass/http/HttpServer.java 21 21 100.00%
🔵 io/questdb/cairo/security/DenyAllSecurityContext.java 1 1 100.00%
🔵 io/questdb/cairo/DefaultCairoConfiguration.java 1 1 100.00%
🔵 io/questdb/cutlass/line/http/LineHttpSender.java 2 2 100.00%
🔵 io/questdb/cairo/security/ReadOnlySecurityContext.java 1 1 100.00%
🔵 io/questdb/cairo/CairoEngine.java 4 4 100.00%
🔵 io/questdb/PropertyKey.java 1 1 100.00%
🔵 io/questdb/cutlass/http/processors/LineHttpProcessorImpl.java 4 4 100.00%
🔵 io/questdb/cutlass/http/HttpMultipartContentParser.java 9 9 100.00%
🔵 io/questdb/cutlass/http/processors/TableStatusCheckProcessor.java 1 1 100.00%
🔵 io/questdb/cairo/CairoConfiguration.java 1 1 100.00%
🔵 io/questdb/cutlass/Services.java 4 4 100.00%
🔵 io/questdb/cutlass/http/ChunkedContentParser.java 1 1 100.00%
🔵 io/questdb/cutlass/http/processors/PrometheusMetricsProcessor.java 1 1 100.00%
🔵 io/questdb/cairo/CairoConfigurationWrapper.java 2 2 100.00%
🔵 io/questdb/cutlass/http/processors/WarningsProcessor.java 1 1 100.00%
🔵 io/questdb/cutlass/http/HttpPostPutProcessor.java 1 1 100.00%
🔵 io/questdb/cutlass/http/processors/SettingsProcessorState.java 10 10 100.00%
🔵 io/questdb/cairo/security/AllowAllSecurityContext.java 1 1 100.00%
🔵 io/questdb/cutlass/http/processors/TextQueryProcessor.java 2 2 100.00%
🔵 io/questdb/cairo/CairoException.java 12 12 100.00%

@bluestreak01 bluestreak01 merged commit 3c4e38a into master May 22, 2025
37 checks passed
@bluestreak01 bluestreak01 deleted the ia_config_endpoint branch May 22, 2025 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants