Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Make write_lp work for both multitenant and singletenant #24422

Closed
wants to merge 1 commit into from

Conversation

mkmik
Copy link
Contributor

@mkmik mkmik commented Oct 19, 2023

(cherry pick of https://github.com/influxdata/influxdb_iox/pull/9027)


Users of the rust influxdb client library should be able to write to InfluxDB serverless multi-tenant and InfluxDB clustered single-tenant databases.

However, the write_lp function was assuming that the caller encoded <org>_<bucket> in the namespace/database name. This encoding is an internal hack of the InfluxDB serverless iox engine. No real clients are sending requests with that encoding over the wire. This encoding is only used internally in serverless to create org scoped databases in an otherwise flat namespace of database names.

This PR makes a source-compatible changes to the write_lp signature that allows users to keep passing database names to write_lp when targeting single-tenant databases, while also allowing the user to explicitly pass an "org" + "database" combo when targeting a multi-tenant database.

This change assumes that most current users of this crate are users of the single-tenant flavour of influxdb

This change does NOT change the behaviour of the influxdb_iox write CLI command, which interprets the namespace parameter as "org_namespace". This is because:

  1. this PR is meant to fix a user facing issue with customers wanting to use the rust client crate but not being able to write to da
  2. since the influxdb_iox repo is now closed source, the write client embedded in the influxdb_iox binary is mostly useful internally by us, and it makes sense to keep it using the same names as they appear in the namespace table in the catalog. The situation may change in the future, when we provide an update influxdb CLI for end-users.

Noise

This PR added a function the returns Result<Foo, Error> and clippy complained with:

    Checking influxdb_iox_client v0.1.0 (/Users/mkm/w/iox/influxdb_iox_client)
error: the `Err`-variant returned from this function is very large
  --> influxdb_iox_client/src/client/write.rs:44:56
   |
44 |     pub fn split_org_db(namespace: impl AsRef<str>) -> Result<Self, Error> {
   |                                                        ^^^^^^^^^^^^^^^^^^^
   |
  ::: influxdb_iox_client/src/client/error.rs:77:5
   |
77 |     NotFound(ServerError<NotFound>),
   |     ------------------------------- the largest variant contains at least 128 bytes
...
80 |     AlreadyExists(ServerError<AlreadyExists>),
   |     ----------------------------------------- the variant `AlreadyExists` contains at least 128 bytes
   |
   = help: try reducing the size of `client::error::Error`, for example by boxing large elements or replacing it with `Box<client::error::Error>`
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#result_large_err
   = note: `-D clippy::result-large-err` implied by `-D warnings`

I'm not sure why it accepted such a large Error enum before, wince write_lp also returns the Error, but anyway, I boxed a few of the nested errors

@mkmik mkmik requested a review from pauldix October 19, 2023 15:26
@pauldix
Copy link
Member

pauldix commented Feb 1, 2024

This got pulled in elsewhere in #24605

@pauldix pauldix closed this Feb 1, 2024
@mgattozzi mgattozzi deleted the write_lp branch March 28, 2024 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants