feat: Implement WAL plugin test API #25704

pauldix · 2024-12-23T00:25:15Z

This implements the WAL plugin test API. It also introduces a new API for the Python plugins to be called, get their data, and call back into the database server.

There are some things that I'll want to address in follow on work:

CLI tests, but will wait on feat: Cleanup CLI flags for InfluxDB 3 Core #25737 to land for a refactor of the CLI here
It uses a _testdb that it injects into the catalog, which might run afoul of the db limits, so need to get around that
Would be better to hook the Python logging to call back into the plugin return state like here: https://pyo3.rs/v0.23.3/ecosystem/logging.html#the-python-to-rust-direction
We should only load the LineBuilder interface once in a module, rather than on every execution of a WAL plugin'
Ideally, we'd be able to validate each line on the write and write_to_db part of the APIs so that a proper error can be returned to the Python code so that plugin authors would be able to handle them. But that will require moving the validator out of the influxdb3_write crate so it's a much larger piece of work
More tests all around

But I want to get this in so that the actual plugin and trigger system can get udated to build around this model.

jacksonrnewhouse

Looks good. Had a couple of questions as it seems to indicate a direction we're heading. One spelling mistake I caught.

jacksonrnewhouse · 2025-01-06T17:16:12Z

influxdb3/src/commands/plugin_test/wal.rs

+    /// If given, save the output to this file on the server in `<plugin-dir>/<name>_test/<save-output-to-file>`
+    #[clap(long = "save-output-to-file")]
+    pub save_output_to_file: Option<String>,
+    /// If given, validate the output against this file on the server in `<plugin-dir>/<name>_test/<validate-output-file>`


In end-to-end tests like this I've often seen an option on whether the system should sort the output data before comparing or not. Think this is worth adding? I do expect the execution to be sequential so it might not matter.

I'd expect the execution to be sequential, but if they end up doing multi-threaded Python maybe things could arrive in different orders? Something to worry about for later I think.

jacksonrnewhouse · 2025-01-06T17:27:23Z

influxdb3/src/commands/plugin_test/wal.rs

+    pub save_output_to_file: Option<String>,
+    /// If given, validate the output against this file on the server in `<plugin-dir>/<name>_test/<validate-output-file>`
+    #[clap(long = "validate-output-file")]
+    pub validate_output_file: Option<String>,


This and save_output_to_file don't actually seem to be used yet. Is that intended?

oh yes, not actually wired up yet. I can remove because I don't think we'll have time to add before release.

jacksonrnewhouse · 2025-01-06T17:30:12Z

influxdb3_write/src/write_buffer/plugins.rs

+    use influxdb3_wal::Gen1Duration;
+
+    // parse the lp into a write batch
+    let namespace = NamespaceName::new("_testdb").unwrap();


Let's factor "_testdb" out into a static variable.

jacksonrnewhouse · 2025-01-06T17:32:09Z

influxdb3_py_api/src/system_py.rs

+}
+
+#[derive(Debug, Default)]
+pub struct PluginReturnState {


Do we want this to include errors? Currently it is constructed assuming successful execution of the plugin code, and then validation errors are tacked on.

Yeah, it would be great to capture errors in the Python here. Although I'm not sure where to do that. Something for follow on work.

jacksonrnewhouse · 2025-01-06T18:54:05Z

influxdb3_write/src/write_buffer/mod.rs

@@ -171,6 +183,7 @@ pub struct WriteBufferImplArgs {
    pub wal_config: WalConfig,
    pub parquet_cache: Option<Arc<dyn ParquetCacheOracle>>,
    pub metric_registry: Arc<Registry>,
+    pub plugin_dir: Option<PathBuf>,


What's the plan here? Prior to this change we were just storing the python file as a string. Do we want the plugin dir to be mirrored to S3?

For now this is just for local development (i.e. I have InfluxDB running on my laptop and I'm working on some_plugin.py file that I want the server to just read in quickly to run the test). Users will deploy plugins by using the API or CLI. Although if we find that users would rather have a directory and deploy files to that dir themselves, we could have that as an option.

jacksonrnewhouse · 2025-01-06T18:55:03Z

influxdb3_py_api/src/system_py.rs

+// constant for the process writes call site string
+const PROCESS_WRITES_CALL_SITE: &str = "process_writes";
+
+const LINE_BUILDER_CODE: &str = r#"


Is this a stop-gap for now so the line builder doesn't need to be installed?

I didn't want to use the existing influxdb3 Python client as it's geared towards making remote requests. I just wanted the simplest possible thing to have some Python lib already in the runtime so that plugin authors could use it.

I think there's going to be some fixed, built-in Python API that we ship with the runtime. For everything else it can be an external library that we can install. Ideally the server would install dependencies for the user automatically when they load the plugin.

jacksonrnewhouse · 2025-01-06T18:55:26Z

influxdb3_write/src/write_buffer/plugins.rs

+            )])),
+        };
+
+        let reesponse =


Spelling: should be response.

This implements the WAL plugin test API. It also introduces a new API for the Python plugins to be called, get their data, and call back into the database server. There are some things that I'll want to address in follow on work: * CLI tests, but will wait on #25737 to land for a refactor of the CLI here * Would be better to hook the Python logging to call back into the plugin return state like here: https://pyo3.rs/v0.23.3/ecosystem/logging.html#the-python-to-rust-direction * We should only load the LineBuilder interface once in a module, rather than on every execution of a WAL plugin * More tests all around But I want to get this in so that the actual plugin and trigger system can get udated to build around this model.

pauldix force-pushed the pd/plugin-development branch 3 times, most recently from e43581e to f5e251e Compare January 5, 2025 21:53

pauldix changed the title ~~WIP: plugin development flow~~ feat: Implement WAL plugin test API Jan 5, 2025

pauldix marked this pull request as ready for review January 5, 2025 21:56

pauldix force-pushed the pd/plugin-development branch from f5e251e to 068d0d8 Compare January 5, 2025 22:00

jacksonrnewhouse approved these changes Jan 6, 2025

View reviewed changes

pauldix force-pushed the pd/plugin-development branch from 80c33ab to 0798fe8 Compare January 6, 2025 22:11

pauldix added 2 commits January 6, 2025 17:11

refactor: PR feedback

20dcd32

pauldix force-pushed the pd/plugin-development branch from 0798fe8 to 20dcd32 Compare January 6, 2025 22:11

pauldix merged commit 1ce6a24 into main Jan 6, 2025
13 checks passed

pauldix deleted the pd/plugin-development branch January 6, 2025 22:32

pauldix mentioned this pull request Jan 8, 2025

Update plugin and WAL trigger to use new structure #25763

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Implement WAL plugin test API #25704

feat: Implement WAL plugin test API #25704

pauldix commented Dec 23, 2024 •

edited

Loading

jacksonrnewhouse left a comment

jacksonrnewhouse Jan 6, 2025

pauldix Jan 6, 2025

jacksonrnewhouse Jan 6, 2025

pauldix Jan 6, 2025

jacksonrnewhouse Jan 6, 2025

jacksonrnewhouse Jan 6, 2025

pauldix Jan 6, 2025

jacksonrnewhouse Jan 6, 2025

pauldix Jan 6, 2025

jacksonrnewhouse Jan 6, 2025

pauldix Jan 6, 2025

jacksonrnewhouse Jan 6, 2025

feat: Implement WAL plugin test API #25704

feat: Implement WAL plugin test API #25704

Conversation

pauldix commented Dec 23, 2024 • edited Loading

jacksonrnewhouse left a 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

pauldix commented Dec 23, 2024 •

edited

Loading