influxdata / influxdata/influxdb

Application of catalog batches should be infallible

Open
#26,504 1 comment 1 reaction 0 assignees View on GitHub
v3
Dominant language
Rust
Stars
31.7k
Forks
3.7k
Avg merge
13h 37m
Merged PRs (30d)
8

Description

### Problem

There is a method in the catalog whose API is fallible, i.e., returns `Result`: https://github.com/influxdata/influxdb/blob/fa646a6f64d0b022ec5c325dba802e8a49be62f8/influxdb3_catalog/src/catalog.rs#L978-L983

This API should not be fallible, because, it is invoked after the provided `CatalogBatch` has been persisted to object store. Therefore, the `CatalogBatch` should already have been validated, and applying it to the in-memory catalog _must_ succeed.

If application does not succeed, then the code should `panic!`, because things are in an unexpected state.

Furthermore, having that and other `apply_*` methods return `Result` indicates to future developers that those methods are allowed to fail, which may lead to bugs.

### Proposed solution

Change the following APIs to not return `Result`, and panic on fallible operations:
* [`apply_catalog_batch`](https://github.com/influxdata/influxdb/blob/fa646a6f64d0b022ec5c325dba802e8a49be62f8/influxdb3_catalog/src/catalog.rs#L979) - a comment/note should be added to this method explaining why it is not fallible
* [`apply_node_batch`](https://github.com/influxdata/influxdb/blob/fa646a6f64d0b022ec5c325dba802e8a49be62f8/influxdb3_catalog/src/catalog.rs#L1002)
* [`apply_token_batch`](https://github.com/influxdata/influxdb/blob/fa646a6f64d0b022ec5c325dba802e8a49be62f8/influxdb3_catalog/src/catalog.rs#L1065)
* [`apply_database_batch`](https://github.com/influxdata/influxdb/blob/fa646a6f64d0b022ec5c325dba802e8a49be62f8/influxdb3_catalog/src/catalog.rs#L1107)

### Additional context

See conversation starting with https://github.com/influxdata/influxdb_pro/pull/910#discussion_r2132850379.

Contributor guide

Open the contributing guide

Research direction

Start in influxdb3_catalog/src/catalog.rs at apply_catalog_batch and the three related apply_*_batch methods. Trace their callers and the existing fallible operations, then verify the catalog crate still builds and its tests pass; done means these APIs no longer return Result, unexpected failures panic, and apply_catalog_batch documents why.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
databases
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.