Skip to content

Commit 233a70d

Browse files
committed
adapter: Messaging for disabling autocommit
Return a more informative error message when a client disables autocommit. This CL changes the logging to only log when disabling automcommit. This CL also makes `Backend::handle_set()` more explicit with respect to the different `UnsupportedSetMode` states. Fixes: REA-4860 Fixes: #1384 Release-Note-Core: Better messaging when clients disable autocommit. Change-Id: Id8def5cfd6d718178746c5599eaa469f872fbe37 Reviewed-on: https://gerrit.readyset.name/c/readyset/+/8086 Reviewed-by: Johnathan Davis <jcd@readyset.io> Tested-by: Buildkite CI
1 parent 1910dc5 commit 233a70d

File tree

2 files changed

+34
-15
lines changed

2 files changed

+34
-15
lines changed

readyset-adapter/src/backend.rs

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2709,26 +2709,34 @@ where
27092709
}
27102710
}
27112711
SetBehavior::Proxy => { /* Do nothing (the caller will proxy for us) */ }
2712-
SetBehavior::SetAutocommit(on) => {
2713-
warn!(
2714-
// FIXME(REA-2168): Use correct dialect.
2715-
set = %set.display(nom_sql::Dialect::MySQL),
2716-
"received unsupported SET statement"
2717-
);
2712+
SetBehavior::SetAutocommit(enabled) => {
2713+
if !enabled {
2714+
warn!(
2715+
// FIXME(REA-2168): Use correct dialect.
2716+
set = %set.display(nom_sql::Dialect::MySQL),
2717+
"Disabling autocommit is an anti-pattern for use with Readyset, as all queries would then be proxied upstream."
2718+
);
2719+
}
2720+
27182721
match settings.unsupported_set_mode {
2719-
UnsupportedSetMode::Error if !on => {
2720-
let e = ReadySetError::SetDisallowed {
2721-
statement: query.to_string(),
2722-
};
2723-
if upstream.is_some() {
2724-
event.set_noria_error(&e);
2722+
UnsupportedSetMode::Error => {
2723+
if enabled {
2724+
state.proxy_state.set_autocommit(enabled);
2725+
} else {
2726+
let e = ReadySetError::DisableAutocommit {
2727+
statement: query.to_string(),
2728+
};
2729+
if upstream.is_some() {
2730+
event.set_noria_error(&e);
2731+
}
2732+
return Err(e.into());
27252733
}
2726-
return Err(e.into());
27272734
}
27282735
UnsupportedSetMode::Proxy => {
2729-
state.proxy_state.set_autocommit(on);
2736+
state.proxy_state.set_autocommit(enabled);
27302737
}
2731-
_ => {}
2738+
// TODO: I'm not sure this is correct ....
2739+
UnsupportedSetMode::Allow => {}
27322740
}
27332741
}
27342742
SetBehavior::SetSearchPath(search_path) => {

readyset-errors/src/lib.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,17 @@ pub enum ReadySetError {
100100
statement: String,
101101
},
102102

103+
/// The adapter will return this error on any set statement that disables
104+
/// MySQL autocommit.
105+
#[error(
106+
"Disabling autocommit is an anti-pattern for use with Readyset, as all queries would then be proxied upstream: {}",
107+
Sensitive(statement)
108+
)]
109+
DisableAutocommit {
110+
/// The set statement passed to the mysql adapter
111+
statement: String,
112+
},
113+
103114
/// Could not connect to the upstream database provided
104115
#[error("Could not connect to the upstream database provided")]
105116
InvalidUpstreamDatabase,

0 commit comments

Comments
 (0)