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

glasstiger · 2025-04-24T01:00:22Z

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

…ig_endpoint

bluestreak01 · 2025-05-07T22:58:34Z

core/src/main/java/io/questdb/cairo/CairoConfiguration.java

@@ -729,7 +733,7 @@ default boolean isValidateSampleByFillType() {
     */
    boolean mangleTableDirNames();

-    default void populateSettings(CharSequenceObjHashMap<CharSequence> settings) {
+    default void populateSettings(Utf8StringSink sink) {


could we please have an interface here, such as Utf8Sink rather than a specific class. Also "populateSettings" is ambiguous name:

lets have JavaDoc here

lets rename the method, using terminology that makes "direction" unambiguous (are we populating config from sink or sink from config?)

renamed method in 1eb812a

bluestreak01 · 2025-05-07T23:00:15Z

core/src/main/java/io/questdb/preferences/PreferencesParser.java

+    private int state = STATE_START;
+
+    public PreferencesParser(CairoConfiguration configuration, CharSequenceObjHashMap<CharSequence> parserMap) {
+        super(16384, configuration.getPreferencesParserCacheSizeLimit(), configuration.getPreferencesStringPoolCapacity());


if 2 of 3 settings are taken from config, why the initial cache size is hardcoded?

the thinking here was that instead of adding yet another config property (which will be barely used, if ever) we can just start with something sensible, and it will expand if it has to.

bluestreak01 · 2025-05-07T23:03:44Z

core/src/main/java/io/questdb/cutlass/http/processors/PreferencesProcessorState.java

+
+    @Override
+    public void close() throws IOException {
+        sink.close();


we have QuietCloseable and IOException is usually not thrown

addressed in 7a56017

bluestreak01 · 2025-05-07T23:05:04Z

core/src/main/java/io/questdb/cutlass/http/processors/PreferencesProcessor.java

+            preferencesStore.save(transientState.sink, mode, version);
+            sendOk();
+        } catch (JsonException | CairoError e) {
+            LOG.error().$("error while saving config").$((Throwable) e).$();


nit: terminology + formatting "could not" or "cannot" - for consistency

addressed in 013292a

bluestreak01 · 2025-05-07T23:09:31Z

core/src/main/java/io/questdb/cutlass/http/processors/PreferencesProcessor.java

+            preferencesStore.save(transientState.sink, mode, version);
+            sendOk();
+        } catch (JsonException | CairoError e) {
+            LOG.error().$("error while saving config").$((Throwable) e).$();


erroneous behaviour is not tested, also missing input fragmentation test

added fragmentation test in 9e6798b

negative tests are in d6b14dd

bluestreak01 · 2025-05-07T23:27:13Z

core/src/main/java/io/questdb/cutlass/http/HttpFullFatServerConfiguration.java

@@ -59,6 +59,10 @@ default ObjList<String> getContextPathImport() {
        return new ObjList<>("/imp");
    }

+    default ObjList<String> getContextPathPreferences() {
+        return new ObjList<>("/preferences");


this is also quite funky to have "preferences" and "settings", those are synonyms. More standard approach would be to have single URL /settings. To save settings we would send POST /settings and to read GET /settings

/preferences has been removed.
/settings handles GET, POST and PUT.
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

bluestreak01 · 2025-05-07T23:28:36Z

core/src/main/java/io/questdb/cutlass/json/AbstractJsonParser.java

+import io.questdb.std.str.AbstractCharSequence;
+import io.questdb.std.str.DirectUtf8Sink;
+
+public abstract class AbstractJsonParser implements JsonParser, Mutable, QuietCloseable {


why do we have abstract class? Preferences parser is the only descendant

ent uses this class too

a comment has been added to this class that it should not be removed

bluestreak01 · 2025-05-07T23:30:35Z

core/src/main/java/io/questdb/griffin/SqlParser.java

@@ -964,8 +964,7 @@ private ExecutionModel parseCreateMatView(
                }
            }

-            final String matViewSql = Chars.toString(lexer.getContent(), startOfQuery, endOfQuery);
-            tableOpBuilder.setSelectText(matViewSql, startOfQuery);
+            tableOpBuilder.setSelectText(lexer.getContent().subSequence(startOfQuery, endOfQuery), startOfQuery);


this is a lot less explicit that toString and thus will be error prone. tableOp is a detachable object, it cannot depend on mutable objects. subSequence() returns potentially mutable interface

changed this back in a57460a

bluestreak01 · 2025-05-07T23:37:19Z

core/src/test/java/io/questdb/test/preferences/PreferencesStoreConcurrentTest.java

+                                assertEquals("\"preferences\":{\"key1\":\"value1\",\"key2\":\"value2\",\"key3\":\"value3\"}", sink);
+                            }
+                        } catch (Throwable th) {
+                            errors.put(threadIndex, th);


reader and writer threads will be overwriting each other errors

fixed in afd5fc4

bluestreak01 · 2025-05-07T23:40:27Z

core/src/main/java/io/questdb/preferences/PreferencesStore.java

+        }
+    }
+
+    public synchronized void populateSettings(Utf8StringSink sink) {


svae/populateSettings are not obviously symmetrical method names

they are not meant to be symmetrical.
for save() the symmetrical method is load().

glasstiger · 2025-05-14T12:19:41Z

[PR Coverage check]

😍 pass : 442 / 456 (96.93%)

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/cutlass/http/processors/LineHttpProcessorImpl.java	4	6	66.67%
🔵	io/questdb/cutlass/http/processors/RejectProcessor.java	2	3	66.67%
🔵	io/questdb/cutlass/json/AbstractJsonParser.java	42	45	93.33%
🔵	io/questdb/cutlass/http/processors/SettingsProcessor.java	60	62	96.77%
🔵	io/questdb/preferences/PreferencesStore.java	72	74	97.30%
🔵	io/questdb/preferences/PreferencesMap.java	60	61	98.36%
🔵	io/questdb/PropServerConfiguration.java	18	18	100.00%
🔵	io/questdb/cutlass/http/HttpHeaderParser.java	10	10	100.00%
🔵	io/questdb/cutlass/http/HttpRequestProcessor.java	1	1	100.00%
🔵	io/questdb/ServerConfiguration.java	6	6	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/http/processors/JsonQueryProcessor.java	1	1	100.00%
🔵	io/questdb/cutlass/http/HttpServer.java	21	21	100.00%
🔵	io/questdb/cairo/DefaultCairoConfiguration.java	2	2	100.00%
🔵	io/questdb/cairo/CairoEngine.java	4	4	100.00%
🔵	io/questdb/PropertyKey.java	1	1	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	3	3	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/HttpRequestValidator.java	38	38	100.00%
🔵	io/questdb/cutlass/http/processors/SettingsProcessorState.java	8	8	100.00%
🔵	io/questdb/cutlass/http/processors/TextQueryProcessor.java	2	2	100.00%
🔵	io/questdb/cairo/CairoException.java	12	12	100.00%

glasstiger added 4 commits April 24, 2025 01:50

chore(core): config endpoint

01e821d

delete unused classes

81a2c6b

remove toString()

8a97ed5

fix tests

9f8eb58

glasstiger marked this pull request as draft April 24, 2025 01:00

glasstiger added the tandem label Apr 29, 2025

glasstiger mentioned this pull request Apr 29, 2025

feat(web-console): instance naming UI questdb/ui#427

Draft

glasstiger and others added 15 commits April 29, 2025 12:36

Merge remote-tracking branch 'origin/master' into ia_config_endpoint

7ffa39c

Merge branch 'master' into ia_config_endpoint

a659353

config parser and store

7c5a5e5

Merge remote-tracking branch 'origin/ia_config_endpoint' into ia_conf…

dc27d3e

…ig_endpoint

get rid of string allocations

4b5be18

Merge branch 'master' into ia_config_endpoint

6b6d678

fix compile error

7538c00

merge mode

ca158a8

concurrent test

3ea741d

Merge remote-tracking branch 'origin/master' into ia_config_endpoint

f836265

add versioning

7b37562

Merge branch 'master' into ia_config_endpoint

6122526

fix tests

72da03e

rename config to preferences

f84c6a8

refactored settings endpoint payload

5a64244

glasstiger marked this pull request as ready for review May 6, 2025 19:36

glasstiger added 2 commits May 7, 2025 10:40

authorization, system admin

11eabff

move hardcoded constant into config

9703862

bluestreak01 requested changes May 7, 2025

View reviewed changes

glasstiger added 4 commits May 8, 2025 10:39

Merge remote-tracking branch 'origin/master' into ia_config_endpoint

729bb6f

populateSettings() -> appendToSettingsSink()

1eb812a

Closable -> QuietCloseable

7a56017

change error message

013292a

glasstiger added 23 commits May 8, 2025 12:54

onContent() -> onChunk()

254cc0b

revert unrelated mat view change back

a57460a

fix read/write keys in error map

afd5fc4

support for different auth type per HTTP method

7fe7cf0

populateSettings() -> appendToSettingsSink()

80728c7

PUT - overwrite, POST - merge

43a30d0

remove Closable

c890a53

move request header checks from context into processor

d82aeda

move request header checks from context into processor

a185b64

abstract json parser comment

e316277

http request handler and validation

1afdd87

code cleanup

3e0ae7f

fix test

0e8ecab

Merge remote-tracking branch 'origin/master' into ia_config_endpoint

262964d

fix test

31ca2f2

fix ent tests

a57cf5a

fixing tests

0c5264f

fragmentation test

9e6798b

fragmentation test

4e8f16a

fragmentation test

a52cf62

improved tests, negative tests, fixes

d6b14dd

persist version

452617a

remove http.context.preferences

60f191b

glasstiger changed the title ~~chore(core): config endpoint~~ chore(core): add preferences to /settings May 14, 2025

glasstiger and others added 2 commits May 14, 2025 12:44

test coverage for default implementations of config interfaces

01ea6fb

Merge branch 'master' into ia_config_endpoint

ea17002

glasstiger requested a review from bluestreak01 May 14, 2025 11:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

glasstiger commented Apr 24, 2025 •

edited

Loading

bluestreak01 May 7, 2025

glasstiger May 8, 2025

bluestreak01 May 7, 2025

glasstiger May 8, 2025

bluestreak01 May 7, 2025

glasstiger May 8, 2025

bluestreak01 May 7, 2025

glasstiger May 8, 2025

bluestreak01 May 7, 2025

glasstiger May 13, 2025

bluestreak01 May 7, 2025

glasstiger May 14, 2025

bluestreak01 May 7, 2025

glasstiger May 8, 2025

glasstiger May 14, 2025

bluestreak01 May 7, 2025

glasstiger May 8, 2025

bluestreak01 May 7, 2025

glasstiger May 8, 2025

bluestreak01 May 7, 2025

glasstiger May 8, 2025

glasstiger commented May 14, 2025

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

Are you sure you want to change the base?

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

Conversation

glasstiger commented Apr 24, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glasstiger commented May 14, 2025

[PR Coverage check]

file detail

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

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

glasstiger commented Apr 24, 2025 •

edited

Loading