Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
42f87f7
Add methods to parse VACUUM statements
CGodiksen Aug 17, 2025
393d0d9
Add support for VACUUM in SQL parser
CGodiksen Aug 17, 2025
885df79
Add support for setting retention period in seconds in configuration
CGodiksen Aug 18, 2025
5e711cb
Add retention period to configuration protocol buffer
CGodiksen Aug 18, 2025
c983fa9
Add test for setting retention period in configuration
CGodiksen Aug 18, 2025
bd039a8
Add integration test for updating retention period
CGodiksen Aug 18, 2025
b803c07
Add method to DeltaLake to vacuum delta table
CGodiksen Aug 18, 2025
a49afe5
Add context method to vacuum a table
CGodiksen Aug 18, 2025
f73eadb
Add utility test functions to create table and write to it
CGodiksen Aug 18, 2025
f7e469e
Add test for vacuuming normal tables
CGodiksen Aug 18, 2025
32dfef6
Add test for vacuuming missing tables
CGodiksen Aug 18, 2025
a1ad409
Add test for vacuuming time series tables
CGodiksen Aug 18, 2025
eb090f0
Add integration test for vacuuming missing table
CGodiksen Aug 18, 2025
2291cb0
Add integration test for vacuuming normal tables
CGodiksen Aug 18, 2025
a82469a
Add integration test for vacuuming time series tables
CGodiksen Aug 18, 2025
11e035d
Add handler for Vacuum statements in manager
CGodiksen Aug 18, 2025
6a843b1
Vacuum all tables if no table names are provided
CGodiksen Aug 18, 2025
c98bcb4
Add support for Vacuum to Operations
CGodiksen Aug 18, 2025
7ba8d12
Add test for vacuuming missing table
CGodiksen Aug 18, 2025
ed56dbc
Add test for vacuuming normal tables
CGodiksen Aug 18, 2025
6b6893f
Add test for vacuuming time series tables
CGodiksen Aug 18, 2025
55382ca
Add functions to C-API to support Vacuum
CGodiksen Aug 18, 2025
163d977
Add method to Python bindings to support vacuum
CGodiksen Aug 18, 2025
8f616fd
Add Python unit tests for data folder vacuum
CGodiksen Aug 18, 2025
45a5f39
Fix clippy issues and run Rustfmt
CGodiksen Aug 18, 2025
d67dfc0
Remove extra check from context vacuum tests
CGodiksen Aug 19, 2025
4d97689
Use token field when checking for EOF
CGodiksen Aug 19, 2025
a9dfdd8
Use variable in configuration tests
CGodiksen Aug 20, 2025
c7e2a25
Remove TABLE_NAME constant from integration tests
CGodiksen Aug 20, 2025
7be5938
Update based on comments from @skejserjensen
CGodiksen Aug 20, 2025
e95eeda
Update based on comments from @chrthomsen
CGodiksen Aug 21, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 17 additions & 0 deletions crates/modelardb_embedded/bindings/python/modelardb/operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,10 @@ def __find_library(build: str) -> str:
int modelardb_embedded_drop(void* maybe_operations_ptr,
bool is_data_folder,
char* table_name_ptr);

int modelardb_embedded_vacuum(void* maybe_operations_ptr,
bool is_data_folder,
char* table_name_ptr);

char* modelardb_embedded_error();
"""
Expand Down Expand Up @@ -724,6 +728,19 @@ def drop(self, table_name: str):
)
self.__check_return_code_and_raise_error(return_code)

def vacuum(self, table_name: str):
"""Vacuum the table with `table_name`.

:param table_name: The name of the table to vacuum.
:type table_name: str
:raises ValueError: If incorrect arguments are provided.
"""
table_name_ptr = ffi.new("char[]", bytes(table_name, "UTF-8"))
return_code = self.__library.modelardb_embedded_vacuum(
self.__operations_ptr, self.__is_data_folder, table_name_ptr
)
self.__check_return_code_and_raise_error(return_code)

def __check_return_code_and_raise_error(self, return_code: int):
"""Raises an appropriate exception based on the return code.

Expand Down
31 changes: 31 additions & 0 deletions crates/modelardb_embedded/bindings/python/tests/test_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,37 @@ def test_data_folder_drop_error(self):
)
self.assertEqual(error_message, str(context.exception))

def test_data_folder_vacuum(self):
with TemporaryDirectory() as temp_dir:
os.environ["MODELARDBD_RETENTION_PERIOD_IN_SECONDS"] = "0"

data_folder = Operations.open_local(temp_dir)
create_tables_in_data_folder(data_folder)

data_folder.write(TIME_SERIES_TABLE_NAME, time_series_table_data())
data_folder.truncate(TIME_SERIES_TABLE_NAME)

# The files should still exist on disk even though they are no longer active.
folder_path = os.path.join(temp_dir, "tables", TIME_SERIES_TABLE_NAME, "field_column=2")
file_count = len(os.listdir(folder_path))
self.assertEqual(file_count, 1)

data_folder.vacuum(TIME_SERIES_TABLE_NAME)

# No files should remain in the column folder.
file_count = len(os.listdir(folder_path))
self.assertEqual(file_count, 0)

def test_data_folder_vacuum_error(self):
with TemporaryDirectory() as temp_dir:
data_folder = Operations.open_local(temp_dir)

with self.assertRaises(RuntimeError) as context:
data_folder.vacuum(MISSING_TABLE_NAME)

error_message = f"Invalid Argument Error: Table with name '{MISSING_TABLE_NAME}' does not exist."
self.assertEqual(error_message, str(context.exception))


def create_tables_in_data_folder(data_folder: Operations):
table_type = NormalTable(normal_table_schema())
Expand Down
25 changes: 25 additions & 0 deletions crates/modelardb_embedded/src/capi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -932,6 +932,31 @@ unsafe fn drop(
TOKIO_RUNTIME.block_on(modelardb.drop(table_name))
}

/// Vacuums the table with the name in `table_name_ptr` in the [`DataFolder`] or [`Client`] in
/// `maybe_operations_ptr`. Assumes `maybe_operations_ptr` points to a [`DataFolder`] or [`Client`];
/// and `table_name_ptr` points to a valid C string.
#[unsafe(no_mangle)]
pub unsafe extern "C" fn modelardb_embedded_vacuum(
maybe_operations_ptr: *mut c_void,
is_data_folder: bool,
table_name_ptr: *const c_char,
) -> c_int {
let maybe_unit = unsafe { vacuum(maybe_operations_ptr, is_data_folder, table_name_ptr) };
set_error_and_return_code(maybe_unit)
}

/// See documentation for [`modelardb_embedded_vacuum`].
unsafe fn vacuum(
maybe_operations_ptr: *mut c_void,
is_data_folder: bool,
table_name_ptr: *const c_char,
) -> Result<()> {
let modelardb = unsafe { c_void_to_operations(maybe_operations_ptr, is_data_folder)? };
let table_name = unsafe { c_char_ptr_to_str(table_name_ptr)? };

TOKIO_RUNTIME.block_on(modelardb.vacuum(table_name))
}

/// Return a read-only [`*const c_char`] with a human-readable representation of the last error the
/// current thread encountered. The lifetime of the returned [`*const c_char`] ends when
/// [`modelardb_embedded_error()`] is called again. If no errors have occurred, a zero-initialized
Expand Down
9 changes: 9 additions & 0 deletions crates/modelardb_embedded/src/operations/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -357,4 +357,13 @@ impl Operations for Client {

Ok(())
}

/// Vacuum the table with the name in `table_name`. If the table could not be vacuumed,
/// [`ModelarDbEmbeddedError`] is returned.
async fn vacuum(&mut self, table_name: &str) -> Result<()> {
let ticket = Ticket::new(format!("VACUUM {table_name}"));
self.flight_client.do_get(ticket).await?;

Ok(())
}
}
105 changes: 105 additions & 0 deletions crates/modelardb_embedded/src/operations/data_folder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

use std::any::Any;
use std::collections::HashMap;
use std::env;
use std::fmt::{Debug, Formatter, Result as FmtResult};
use std::path::Path as StdPath;
use std::pin::Pin;
Expand Down Expand Up @@ -851,6 +852,24 @@ impl Operations for DataFolder {

Ok(())
}

/// Vacuum the table with the name in `table_name`. If the table does not exist or the
/// table could not be vacuumed, [`ModelarDbEmbeddedError`] is returned.
async fn vacuum(&mut self, table_name: &str) -> Result<()> {
if self.tables().await?.contains(&table_name.to_owned()) {
let retention_period_in_seconds = env::var("MODELARDBD_RETENTION_PERIOD_IN_SECONDS")
.map_or(60 * 60 * 24 * 7, |value| value.parse().unwrap());
Comment thread
skejserjensen marked this conversation as resolved.

self.delta_lake
.vacuum_table(table_name, retention_period_in_seconds)
.await
.map_err(|error| error.into())
} else {
Err(ModelarDbEmbeddedError::InvalidArgument(format!(
"Table with name '{table_name}' does not exist."
)))
}
}
}

/// Sort the `uncompressed_data` from the time series table with `time_series_table_metadata`
Expand Down Expand Up @@ -922,6 +941,8 @@ fn schemas_are_compatible(source_schema: &Schema, target_schema: &Schema) -> boo
mod tests {
use super::*;

use std::sync::{LazyLock, Mutex};

use arrow::array::{Array, Float64Array, Int8Array, Int16Array, Int32Array, Int64Array};
use arrow::datatypes::{ArrowPrimitiveType, DataType, Field};
use arrow_flight::flight_service_client::FlightServiceClient;
Expand All @@ -942,6 +963,9 @@ mod tests {
const TIME_SERIES_TABLE_WITH_GENERATED_COLUMN_NAME: &str = "time_series_table_with_generated";
const INVALID_COLUMN_NAME: &str = "invalid_column";

/// Lock used for env::set_var() as it is not guaranteed to be thread-safe.
static SET_VAR_LOCK: LazyLock<Mutex<()>> = LazyLock::new(|| Mutex::new(()));

#[tokio::test]
async fn test_create_normal_table() {
let (_temp_dir, data_folder) = create_data_folder_with_normal_table().await;
Expand Down Expand Up @@ -2540,6 +2564,87 @@ mod tests {
);
}

#[tokio::test]
async fn test_vacuum_normal_table() {
// env::set_var is safe to call in a single-threaded program.
unsafe {
let _mutex_guard = SET_VAR_LOCK.lock();
env::set_var("MODELARDBD_RETENTION_PERIOD_IN_SECONDS", "0");
}
Comment thread
CGodiksen marked this conversation as resolved.

let (temp_dir, mut data_folder) = create_data_folder_with_normal_table().await;

data_folder
.write(NORMAL_TABLE_NAME, normal_table_data())
.await
.unwrap();

data_folder.truncate(NORMAL_TABLE_NAME).await.unwrap();

// The files should still exist on disk even though they are no longer active.
let table_path = format!(
"{}/tables/{}",
temp_dir.path().to_str().unwrap(),
NORMAL_TABLE_NAME
);
let files = std::fs::read_dir(&table_path).unwrap();
assert_eq!(files.count(), 2);

data_folder.vacuum(NORMAL_TABLE_NAME).await.unwrap();

// Only the _delta_log folder should remain.
let files = std::fs::read_dir(&table_path).unwrap();
assert_eq!(files.count(), 1);
}

#[tokio::test]
async fn test_vacuum_time_series_table() {
// env::set_var is safe to call in a single-threaded program.
unsafe {
let _mutex_guard = SET_VAR_LOCK.lock();
env::set_var("MODELARDBD_RETENTION_PERIOD_IN_SECONDS", "0");
}
Comment thread
CGodiksen marked this conversation as resolved.

let (temp_dir, mut data_folder) = create_data_folder_with_time_series_table().await;

data_folder
.write(TIME_SERIES_TABLE_NAME, time_series_table_data())
.await
.unwrap();

data_folder.truncate(TIME_SERIES_TABLE_NAME).await.unwrap();

// The files should still exist on disk even though they are no longer active.
let column_path = format!(
"{}/tables/{}/field_column=3",
temp_dir.path().to_str().unwrap(),
TIME_SERIES_TABLE_NAME
);
let files = std::fs::read_dir(&column_path).unwrap();
assert_eq!(files.count(), 1);

data_folder.vacuum(TIME_SERIES_TABLE_NAME).await.unwrap();

// No files should remain in the column folder.
let files = std::fs::read_dir(&column_path).unwrap();
assert_eq!(files.count(), 0);
}

#[tokio::test]
async fn test_vacuum_missing_table() {
let temp_dir = tempfile::tempdir().unwrap();
let mut data_folder = DataFolder::open_local(temp_dir.path()).await.unwrap();

let result = data_folder.vacuum(MISSING_TABLE_NAME).await;

assert_eq!(
result.unwrap_err().to_string(),
format!(
"Invalid Argument Error: Table with name '{MISSING_TABLE_NAME}' does not exist."
)
);
}

#[tokio::test]
async fn test_move_normal_table_to_normal_table() {
let (_temp_dir, mut source) = create_data_folder_with_normal_table().await;
Expand Down
3 changes: 3 additions & 0 deletions crates/modelardb_embedded/src/operations/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,9 @@ pub trait Operations: Sync + Send {

/// Drop the table with the name in `table_name`.
async fn drop(&mut self, table_name: &str) -> Result<()>;

/// Vacuum the table with the name in `table_name`.
async fn vacuum(&mut self, table_name: &str) -> Result<()>;
}

/// Use the time series table metadata in `table_name`, `schema`, `error_bounds`, and `generated_columns`
Expand Down
50 changes: 47 additions & 3 deletions crates/modelardb_manager/src/remote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ use std::error::Error;
use std::net::SocketAddr;
use std::pin::Pin;
use std::result::Result as StdResult;
use std::str;
use std::sync::Arc;
use std::{env, str};

use arrow::datatypes::Schema;
use arrow::ipc::writer::IpcWriteOptions;
Expand Down Expand Up @@ -333,6 +333,33 @@ impl FlightServiceHandler {

Ok(())
}

/// Vacuum the table in the remote data folder and at each node controlled by the manager. If
/// the table does not exist or the table cannot be vacuumed in the remote data folder
/// and at each node, return [`Status`].
async fn vacuum_cluster_table(&self, table_name: &str) -> StdResult<(), Status> {
let retention_period_in_seconds = env::var("MODELARDBD_RETENTION_PERIOD_IN_SECONDS")
.map_or(60 * 60 * 24 * 7, |value| value.parse().unwrap());

// Vacuum the table in the remote data folder Delta lake.
self.context
.remote_data_folder
.delta_lake
.vacuum_table(table_name, retention_period_in_seconds)
.await
.map_err(error_to_status_internal)?;

// Vacuum the table in the nodes controlled by the manager.
self.context
.cluster
.read()
.await
.cluster_do_get(&format!("VACUUM {table_name}"), &self.context.key)
.await
.map_err(error_to_status_internal)?;

Ok(())
}
}

#[tonic::async_trait]
Expand Down Expand Up @@ -453,8 +480,8 @@ impl FlightService for FlightServiceHandler {
}

/// Execute a SQL statement provided in UTF-8 and return the schema of the result followed by
/// the result itself. Currently, CREATE TABLE, CREATE TIME SERIES TABLE, TRUNCATE TABLE, and
/// DROP TABLE are supported.
/// the result itself. Currently, CREATE TABLE, CREATE TIME SERIES TABLE, TRUNCATE TABLE,
/// DROP TABLE, and VACUUM are supported.
async fn do_get(
&self,
request: Request<Ticket>,
Expand Down Expand Up @@ -497,6 +524,23 @@ impl FlightService for FlightServiceHandler {
self.drop_cluster_table(&table_name).await?;
}
}
ModelarDbStatement::Vacuum(mut table_names) => {
// Vacuum all tables if no table names are provided.
if table_names.is_empty() {
table_names = self
.context
.remote_data_folder
.metadata_manager
.table_metadata_manager
.table_names()
.await
.map_err(error_to_status_internal)?;
}

for table_name in table_names {
self.vacuum_cluster_table(&table_name).await?;
}
}
// .. is not used so a compile error is raised if a new ModelarDbStatement is added.
ModelarDbStatement::Statement(_) | ModelarDbStatement::IncludeSelect(..) => {
return Err(Status::invalid_argument(
Expand Down
Loading
Loading