Keep plugins persistently running in the background (#12064)

# Description
This PR uses the new plugin protocol to intelligently keep plugin
processes running in the background for further plugin calls.

Running plugins can be seen by running the new `plugin list` command,
and stopped by running the new `plugin stop` command.

This is an enhancement for the performance of plugins, as starting new
plugin processes has overhead, especially for plugins in languages that
take a significant amount of time on startup. It also enables plugins
that have persistent state between commands, making the migration of
features like dataframes and `stor` to plugins possible.

Plugins are automatically stopped by the new plugin garbage collector,
configurable with `$env.config.plugin_gc`:

```nushell
  $env.config.plugin_gc = {
      # Configuration for plugin garbage collection
      default: {
          enabled: true # true to enable stopping of inactive plugins
          stop_after: 10sec # how long to wait after a plugin is inactive to stop it
      }
      plugins: {
          # alternate configuration for specific plugins, by name, for example:
          #
          # gstat: {
          #     enabled: false
          # }
      }
  }
```

If garbage collection is enabled, plugins will be stopped after
`stop_after` passes after they were last active. Plugins are counted as
inactive if they have no running plugin calls. Reading the stream from
the response of a plugin call is still considered to be activity, but if
a plugin holds on to a stream but the call ends without an active
streaming response, it is not counted as active even if it is reading
it. Plugins can explicitly disable the GC as appropriate with
`engine.set_gc_disabled(true)`.

The `version` command now lists plugin names rather than plugin
commands. The list of plugin commands is accessible via `plugin list`.

Recommend doing this together with #12029, because it will likely force
plugin developers to do the right thing with mutability and lead to less
unexpected behavior when running plugins nested / in parallel.

# User-Facing Changes
- new command: `plugin list`
- new command: `plugin stop`
- changed command: `version` (now lists plugin names, rather than
commands)
- new config: `$env.config.plugin_gc`
- Plugins will keep running and be reused, at least for the configured
GC period
- Plugins that used mutable state in weird ways like `inc` did might
misbehave until fixed
- Plugins can disable GC if they need to
- Had to change plugin signature to accept `&EngineInterface` so that
the GC disable feature works. #12029 does this anyway, and I'm expecting
(resolvable) conflicts with that

# Tests + Formatting
- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`

Because there is some specific OS behavior required for plugins to not
respond to Ctrl-C directly, I've developed against and tested on both
Linux and Windows to ensure that works properly.

# After Submitting
I think this probably needs to be in the book somewhere
This commit is contained in:
Devyn Cairns
2024-03-09 15:10:22 -08:00
committed by GitHub
parent 430fb1fcb6
commit bc19be25b1
44 changed files with 2131 additions and 304 deletions

View File

@@ -320,6 +320,16 @@ impl PluginCallResponse<PipelineDataHeader> {
}
}
/// Options that can be changed to affect how the engine treats the plugin
#[derive(Serialize, Deserialize, Debug, Clone)]
pub enum PluginOption {
/// Send `GcDisabled(true)` to stop the plugin from being automatically garbage collected, or
/// `GcDisabled(false)` to enable it again.
///
/// See [`EngineInterface::set_gc_disabled`] for more information.
GcDisabled(bool),
}
/// Information received from the plugin
///
/// Note: exported for internal use, not public.
@@ -328,6 +338,8 @@ impl PluginCallResponse<PipelineDataHeader> {
pub enum PluginOutput {
/// This must be the first message. Indicates supported protocol
Hello(ProtocolInfo),
/// Set option. No response expected
Option(PluginOption),
/// A response to a [`PluginCall`]. The ID should be the same sent with the plugin call this
/// is a response to
CallResponse(PluginCallId, PluginCallResponse<PipelineDataHeader>),

View File

@@ -3,7 +3,7 @@ use std::sync::Arc;
use nu_protocol::{CustomValue, ShellError, Span, Spanned, Value};
use serde::{Deserialize, Serialize};
use crate::plugin::PluginIdentity;
use crate::plugin::PluginSource;
#[cfg(test)]
mod tests;
@@ -17,7 +17,7 @@ mod tests;
/// that local plugin custom values are converted to and from [`PluginCustomData`] on the boundary.
///
/// [`PluginInterface`](crate::interface::PluginInterface) is responsible for adding the
/// appropriate [`PluginIdentity`](crate::plugin::PluginIdentity), ensuring that only
/// appropriate [`PluginSource`](crate::plugin::PluginSource), ensuring that only
/// [`PluginCustomData`] is contained within any values sent, and that the `source` of any
/// values sent matches the plugin it is being sent to.
#[derive(Clone, Debug, Serialize, Deserialize)]
@@ -30,7 +30,7 @@ pub struct PluginCustomValue {
/// Which plugin the custom value came from. This is not defined on the plugin side. The engine
/// side is responsible for maintaining it, and it is not sent over the serialization boundary.
#[serde(skip, default)]
pub source: Option<Arc<PluginIdentity>>,
pub(crate) source: Option<Arc<PluginSource>>,
}
#[typetag::serde]
@@ -52,7 +52,7 @@ impl CustomValue for PluginCustomValue {
"Unable to spawn plugin `{}` to get base value",
self.source
.as_ref()
.map(|s| s.plugin_name.as_str())
.map(|s| s.name())
.unwrap_or("<unknown>")
),
msg: err.to_string(),
@@ -61,14 +61,18 @@ impl CustomValue for PluginCustomValue {
inner: vec![err],
};
let identity = self.source.clone().ok_or_else(|| {
let source = self.source.clone().ok_or_else(|| {
wrap_err(ShellError::NushellFailed {
msg: "The plugin source for the custom value was not set".into(),
})
})?;
let empty_env: Option<(String, String)> = None;
let plugin = identity.spawn(empty_env).map_err(wrap_err)?;
// Envs probably should be passed here, but it's likely that the plugin is already running
let empty_envs = std::iter::empty::<(&str, &str)>();
let plugin = source
.persistent(Some(span))
.and_then(|p| p.get(|| Ok(empty_envs)))
.map_err(wrap_err)?;
plugin
.custom_value_to_base_value(Spanned {
@@ -117,8 +121,8 @@ impl PluginCustomValue {
})
}
/// Add a [`PluginIdentity`] to all [`PluginCustomValue`]s within a value, recursively.
pub(crate) fn add_source(value: &mut Value, source: &Arc<PluginIdentity>) {
/// Add a [`PluginSource`] to all [`PluginCustomValue`]s within a value, recursively.
pub(crate) fn add_source(value: &mut Value, source: &Arc<PluginSource>) {
let span = value.span();
match value {
// Set source on custom value
@@ -179,21 +183,26 @@ impl PluginCustomValue {
/// since `LazyRecord` could return something different the next time it is called.
pub(crate) fn verify_source(
value: &mut Value,
source: &PluginIdentity,
source: &PluginSource,
) -> Result<(), ShellError> {
let span = value.span();
match value {
// Set source on custom value
Value::CustomValue { val, .. } => {
if let Some(custom_value) = val.as_any().downcast_ref::<PluginCustomValue>() {
if custom_value.source.as_deref() == Some(source) {
if custom_value
.source
.as_ref()
.map(|s| s.is_compatible(source))
.unwrap_or(false)
{
Ok(())
} else {
Err(ShellError::CustomValueIncorrectForPlugin {
name: custom_value.name.clone(),
span,
dest_plugin: source.plugin_name.clone(),
src_plugin: custom_value.source.as_ref().map(|s| s.plugin_name.clone()),
dest_plugin: source.name().to_owned(),
src_plugin: custom_value.source.as_ref().map(|s| s.name().to_owned()),
})
}
} else {
@@ -201,7 +210,7 @@ impl PluginCustomValue {
Err(ShellError::CustomValueIncorrectForPlugin {
name: val.value_string(),
span,
dest_plugin: source.plugin_name.clone(),
dest_plugin: source.name().to_owned(),
src_plugin: None,
})
}

View File

@@ -1,9 +1,11 @@
use std::sync::Arc;
use nu_protocol::{
ast::RangeInclusion, engine::Closure, record, CustomValue, Range, ShellError, Span, Value,
};
use crate::{
plugin::PluginIdentity,
plugin::PluginSource,
protocol::test_util::{
expected_test_custom_value, test_plugin_custom_value, test_plugin_custom_value_with_source,
TestCustomValue,
@@ -45,7 +47,7 @@ fn expected_serialize_output() -> Result<(), ShellError> {
#[test]
fn add_source_at_root() -> Result<(), ShellError> {
let mut val = Value::test_custom_value(Box::new(test_plugin_custom_value()));
let source = PluginIdentity::new_fake("foo");
let source = Arc::new(PluginSource::new_fake("foo"));
PluginCustomValue::add_source(&mut val, &source);
let custom_value = val.as_custom_value()?;
@@ -53,7 +55,10 @@ fn add_source_at_root() -> Result<(), ShellError> {
.as_any()
.downcast_ref()
.expect("not PluginCustomValue");
assert_eq!(Some(source), plugin_custom_value.source);
assert_eq!(
Some(Arc::as_ptr(&source)),
plugin_custom_value.source.as_ref().map(Arc::as_ptr)
);
Ok(())
}
@@ -84,7 +89,7 @@ fn add_source_nested_range() -> Result<(), ShellError> {
to: orig_custom_val.clone(),
inclusion: RangeInclusion::Inclusive,
});
let source = PluginIdentity::new_fake("foo");
let source = Arc::new(PluginSource::new_fake("foo"));
PluginCustomValue::add_source(&mut val, &source);
check_range_custom_values(&val, |name, custom_value| {
@@ -93,8 +98,8 @@ fn add_source_nested_range() -> Result<(), ShellError> {
.downcast_ref()
.unwrap_or_else(|| panic!("{name} not PluginCustomValue"));
assert_eq!(
Some(&source),
plugin_custom_value.source.as_ref(),
Some(Arc::as_ptr(&source)),
plugin_custom_value.source.as_ref().map(Arc::as_ptr),
"{name} source not set correctly"
);
Ok(())
@@ -126,7 +131,7 @@ fn add_source_nested_record() -> Result<(), ShellError> {
"foo" => orig_custom_val.clone(),
"bar" => orig_custom_val.clone(),
});
let source = PluginIdentity::new_fake("foo");
let source = Arc::new(PluginSource::new_fake("foo"));
PluginCustomValue::add_source(&mut val, &source);
check_record_custom_values(&val, &["foo", "bar"], |key, custom_value| {
@@ -135,8 +140,8 @@ fn add_source_nested_record() -> Result<(), ShellError> {
.downcast_ref()
.unwrap_or_else(|| panic!("'{key}' not PluginCustomValue"));
assert_eq!(
Some(&source),
plugin_custom_value.source.as_ref(),
Some(Arc::as_ptr(&source)),
plugin_custom_value.source.as_ref().map(Arc::as_ptr),
"'{key}' source not set correctly"
);
Ok(())
@@ -165,7 +170,7 @@ fn check_list_custom_values(
fn add_source_nested_list() -> Result<(), ShellError> {
let orig_custom_val = Value::test_custom_value(Box::new(test_plugin_custom_value()));
let mut val = Value::test_list(vec![orig_custom_val.clone(), orig_custom_val.clone()]);
let source = PluginIdentity::new_fake("foo");
let source = Arc::new(PluginSource::new_fake("foo"));
PluginCustomValue::add_source(&mut val, &source);
check_list_custom_values(&val, 0..=1, |index, custom_value| {
@@ -174,8 +179,8 @@ fn add_source_nested_list() -> Result<(), ShellError> {
.downcast_ref()
.unwrap_or_else(|| panic!("[{index}] not PluginCustomValue"));
assert_eq!(
Some(&source),
plugin_custom_value.source.as_ref(),
Some(Arc::as_ptr(&source)),
plugin_custom_value.source.as_ref().map(Arc::as_ptr),
"[{index}] source not set correctly"
);
Ok(())
@@ -209,7 +214,7 @@ fn add_source_nested_closure() -> Result<(), ShellError> {
block_id: 0,
captures: vec![(0, orig_custom_val.clone()), (1, orig_custom_val.clone())],
});
let source = PluginIdentity::new_fake("foo");
let source = Arc::new(PluginSource::new_fake("foo"));
PluginCustomValue::add_source(&mut val, &source);
check_closure_custom_values(&val, 0..=1, |index, custom_value| {
@@ -218,8 +223,8 @@ fn add_source_nested_closure() -> Result<(), ShellError> {
.downcast_ref()
.unwrap_or_else(|| panic!("[{index}] not PluginCustomValue"));
assert_eq!(
Some(&source),
plugin_custom_value.source.as_ref(),
Some(Arc::as_ptr(&source)),
plugin_custom_value.source.as_ref().map(Arc::as_ptr),
"[{index}] source not set correctly"
);
Ok(())
@@ -233,10 +238,10 @@ fn verify_source_error_message() -> Result<(), ShellError> {
let mut native_val = Value::custom_value(Box::new(TestCustomValue(32)), span);
let mut foreign_val = {
let mut val = test_plugin_custom_value();
val.source = Some(PluginIdentity::new_fake("other"));
val.source = Some(Arc::new(PluginSource::new_fake("other")));
Value::custom_value(Box::new(val), span)
};
let source = PluginIdentity::new_fake("test");
let source = PluginSource::new_fake("test");
PluginCustomValue::verify_source(&mut ok_val, &source).expect("ok_val should be verified ok");
@@ -266,7 +271,7 @@ fn verify_source_error_message() -> Result<(), ShellError> {
#[test]
fn verify_source_nested_range() -> Result<(), ShellError> {
let native_val = Value::test_custom_value(Box::new(TestCustomValue(32)));
let source = PluginIdentity::new_fake("test");
let source = PluginSource::new_fake("test");
for (name, mut val) in [
(
"from",
@@ -315,7 +320,7 @@ fn verify_source_nested_range() -> Result<(), ShellError> {
#[test]
fn verify_source_nested_record() -> Result<(), ShellError> {
let native_val = Value::test_custom_value(Box::new(TestCustomValue(32)));
let source = PluginIdentity::new_fake("test");
let source = PluginSource::new_fake("test");
for (name, mut val) in [
(
"first element foo",
@@ -346,7 +351,7 @@ fn verify_source_nested_record() -> Result<(), ShellError> {
#[test]
fn verify_source_nested_list() -> Result<(), ShellError> {
let native_val = Value::test_custom_value(Box::new(TestCustomValue(32)));
let source = PluginIdentity::new_fake("test");
let source = PluginSource::new_fake("test");
for (name, mut val) in [
(
"first element",
@@ -371,7 +376,7 @@ fn verify_source_nested_list() -> Result<(), ShellError> {
#[test]
fn verify_source_nested_closure() -> Result<(), ShellError> {
let native_val = Value::test_custom_value(Box::new(TestCustomValue(32)));
let source = PluginIdentity::new_fake("test");
let source = PluginSource::new_fake("test");
for (name, mut val) in [
(
"first capture",

View File

@@ -1,7 +1,7 @@
use nu_protocol::{CustomValue, ShellError, Span, Value};
use serde::{Deserialize, Serialize};
use crate::plugin::PluginIdentity;
use crate::plugin::PluginSource;
use super::PluginCustomValue;
@@ -44,7 +44,7 @@ pub(crate) fn expected_test_custom_value() -> TestCustomValue {
pub(crate) fn test_plugin_custom_value_with_source() -> PluginCustomValue {
PluginCustomValue {
source: Some(PluginIdentity::new_fake("test")),
source: Some(PluginSource::new_fake("test").into()),
..test_plugin_custom_value()
}
}