From 5aee25451b4204fe1f36ff7fd067462c10cff360 Mon Sep 17 00:00:00 2001 From: Raphael Taylor-Davies Date: Mon, 22 Feb 2021 10:48:11 +0000 Subject: [PATCH 01/22] feat: gRPC management API definitions --- generated_types/build.rs | 27 +- .../storage/read}/storage_common_idpe.proto | 4 +- .../iox/management/v1/base_types.proto | 30 +++ .../iox/management/v1/database_rules.proto | 251 ++++++++++++++++++ .../iox/management/v1/service.proto | 43 +++ .../platform/storage}/predicate.proto | 0 .../platform/storage}/service.proto | 5 +- .../influxdata/platform/storage}/source.proto | 0 .../platform/storage}/storage_common.proto | 2 +- .../influxdata/platform/storage}/test.proto | 0 generated_types/{ => protos}/wal.fbs | 0 generated_types/src/lib.rs | 58 +++- 12 files changed, 393 insertions(+), 27 deletions(-) rename generated_types/{ => protos/com/github/influxdata/idpe/storage/read}/storage_common_idpe.proto (87%) create mode 100644 generated_types/protos/influxdata/iox/management/v1/base_types.proto create mode 100644 generated_types/protos/influxdata/iox/management/v1/database_rules.proto create mode 100644 generated_types/protos/influxdata/iox/management/v1/service.proto rename generated_types/{ => protos/influxdata/platform/storage}/predicate.proto (100%) rename generated_types/{ => protos/influxdata/platform/storage}/service.proto (93%) rename generated_types/{ => protos/influxdata/platform/storage}/source.proto (100%) rename generated_types/{ => protos/influxdata/platform/storage}/storage_common.proto (99%) rename generated_types/{ => protos/influxdata/platform/storage}/test.proto (100%) rename generated_types/{ => protos}/wal.fbs (100%) diff --git a/generated_types/build.rs b/generated_types/build.rs index 367786400b..b58e708cd3 100644 --- a/generated_types/build.rs +++ b/generated_types/build.rs @@ -10,7 +10,7 @@ type Error = Box; type Result = std::result::Result; fn main() -> Result<()> { - let root = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + let root = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("protos"); generate_grpc_types(&root)?; generate_wal_types(&root)?; @@ -20,16 +20,25 @@ fn main() -> Result<()> { /// Schema used with IOx specific gRPC requests /// -/// Creates `influxdata.platform.storage.rs` and -/// `com.github.influxdata.idpe.storage.read.rs` +/// Creates +/// - `influxdata.platform.storage.rs` +/// - `com.github.influxdata.idpe.storage.read.rs` +/// - `influxdata.iox.management.v1.rs` fn generate_grpc_types(root: &Path) -> Result<()> { + let storage_path = root.join("influxdata/platform/storage"); + let idpe_path = root.join("com/github/influxdata/idpe/storage/read"); + let management_path = root.join("influxdata/iox/management/v1"); + let proto_files = vec![ - root.join("test.proto"), - root.join("predicate.proto"), - root.join("storage_common.proto"), - root.join("storage_common_idpe.proto"), - root.join("service.proto"), - root.join("source.proto"), + storage_path.join("test.proto"), + storage_path.join("predicate.proto"), + storage_path.join("storage_common.proto"), + storage_path.join("service.proto"), + storage_path.join("source.proto"), + idpe_path.join("storage_common_idpe.proto"), + management_path.join("base_types.proto"), + management_path.join("database_rules.proto"), + management_path.join("service.proto"), ]; // Tell cargo to recompile if any of these proto files are changed diff --git a/generated_types/storage_common_idpe.proto b/generated_types/protos/com/github/influxdata/idpe/storage/read/storage_common_idpe.proto similarity index 87% rename from generated_types/storage_common_idpe.proto rename to generated_types/protos/com/github/influxdata/idpe/storage/read/storage_common_idpe.proto index e80ebe590d..da74002ded 100644 --- a/generated_types/storage_common_idpe.proto +++ b/generated_types/protos/com/github/influxdata/idpe/storage/read/storage_common_idpe.proto @@ -10,8 +10,8 @@ syntax = "proto3"; package influxdata.platform.storage; import "google/protobuf/any.proto"; -import "predicate.proto"; -import "storage_common.proto"; +import "influxdata/platform/storage/predicate.proto"; +import "influxdata/platform/storage/storage_common.proto"; message ReadSeriesCardinalityRequest { google.protobuf.Any read_series_cardinality_source = 1; diff --git a/generated_types/protos/influxdata/iox/management/v1/base_types.proto b/generated_types/protos/influxdata/iox/management/v1/base_types.proto new file mode 100644 index 0000000000..2618f44290 --- /dev/null +++ b/generated_types/protos/influxdata/iox/management/v1/base_types.proto @@ -0,0 +1,30 @@ +syntax = "proto3"; +package influxdata.iox.management.v1; + +enum Order { + ORDER_UNKNOWN = 0; + ORDER_ASC = 1; + ORDER_DESC = 2; +} + +enum Aggregate { + AGGREGATE_UNKNOWN = 0; + AGGREGATE_MIN = 1; + AGGREGATE_MAX = 2; +} + +enum ColumnType { + COLUMN_TYPE_UNKNOWN = 0; + COLUMN_TYPE_I64 = 1; + COLUMN_TYPE_U64 = 2; + COLUMN_TYPE_F64 = 3; + COLUMN_TYPE_STRING = 4; + COLUMN_TYPE_BOOL = 5; +} + +message HostGroup { + string id = 1; + + repeated string hosts = 2; +} + diff --git a/generated_types/protos/influxdata/iox/management/v1/database_rules.proto b/generated_types/protos/influxdata/iox/management/v1/database_rules.proto new file mode 100644 index 0000000000..4e9a780a2b --- /dev/null +++ b/generated_types/protos/influxdata/iox/management/v1/database_rules.proto @@ -0,0 +1,251 @@ +syntax = "proto3"; +package influxdata.iox.management.v1; + +import "google/protobuf/duration.proto"; +import "google/protobuf/empty.proto"; +import "influxdata/iox/management/v1/base_types.proto"; + +// `PartitionTemplate` is used to compute the partition key of each row that +// gets written. It can consist of the table name, a column name and its value, +// a formatted time, or a string column and regex captures of its value. For +// columns that do not appear in the input row, a blank value is output. +// +// The key is constructed in order of the template parts; thus ordering changes +// what partition key is generated. +message PartitionTemplate { + message Part { + message ColumnFormat { + string column = 1; + string format = 2; + } + + oneof part { + google.protobuf.Empty table = 1; + string column = 2; + string time = 3; + ColumnFormat regex = 4; + ColumnFormat strfTime = 5; + } + } + + repeated Part parts = 1; +} + +message Matcher { + // Restrict selection to a specific table or table's specified by a regex + oneof tableMatcher { + string table = 1; + string regex = 2; + } + // A query predicate to filter rows + string predicate = 3; +} + +message ReplicationConfig { + message Replication { + string hostGroupId = 1; + } + + // The set of host groups that data should be replicated to. Which host a + // write goes to within a host group is determined by consistent hashing of + // the partition key. We'd use this to create a host group per + // availability zone, so you might have 5 availability zones with 2 + // hosts in each. Replication will ensure that N of those zones get a + // write. For each zone, only a single host needs to get the write. + // Replication is for ensuring a write exists across multiple hosts + // before returning success. Its purpose is to ensure write durability, + // rather than write availability for query (this is covered by + // subscriptions). + repeated string replications = 1; + + // The minimum number of host groups to replicate a write to before success + // is returned. This can be overridden on a per request basis. + // Replication will continue to write to the other host groups in the + // background. + uint32 replicationCount = 2; + + // How long the replication queue can get before either rejecting writes or + // dropping missed writes. The queue is kept in memory on a + // per-database basis. A queue size of zero means it will only try to + // replicate synchronously and drop any failures. + uint64 replicationQueueMaxSize = 3; +} + +message SubscriptionConfig { + message Subscription { + string name = 1; + string hostGroupId = 2; + Matcher matcher = 3; + } + + // `subscriptions` are used for query servers to get data via either push + // or pull as it arrives. They are separate from replication as they + // have a different purpose. They're for query servers or other clients + // that want to subscribe to some subset of data being written in. This + // could either be specific partitions, ranges of partitions, tables, or + // rows matching some predicate. + repeated Subscription subscriptions = 1; +} + +message QueryConfig { + // If set to `true`, this server should answer queries from one or more of + // of its local write buffer and any read-only partitions that it knows + // about. In this case, results will be merged with any others from the + // remote goups or read only partitions. + bool queryLocal = 1; + + // Set `primary` to a host group if remote servers should be + // issued queries for this database. All hosts in the group should be + // queried with this server acting as the coordinator that merges + // results together. + string primary = 2; + + // If a specific host in the primary group is unavailable, + // another host in the same position from a secondary group should be + // queried. For example, imagine we've partitioned the data in this DB into + // 4 partitions and we are replicating the data across 3 availability + // zones. We have 4 hosts in each of those AZs, thus they each have 1 + // partition. We'd set the primary group to be the 4 hosts in the same + // AZ as this one, and the secondary groups as the hosts in the other 2 AZs. + repeated string secondaries = 3; + + // Use `readOnlyPartitions` when a server should answer queries for + // partitions that come from object storage. This can be used to start + // up a new query server to handle queries by pointing it at a + // collection of partitions and then telling it to also pull + // data from the replication servers (writes that haven't been snapshotted + // into a partition). + repeated string readOnlyPartitions = 4; +} + +message WalBufferConfig { + enum Rollover { + ROLLOVER_UNKNOWN = 0; + + // Drop the old segment even though it hasn't been persisted. This part of + // the WAl will be lost on this server. + ROLLOVER_DROP_OLD_SEGMENT = 1; + + // Drop the incoming write and fail silently. This favors making sure that + // older WAL data will be backed up. + ROLLOVER_DROP_INCOMING = 2; + + // Reject the incoming write and return an error. The client may retry the + // request, which will succeed once the oldest segment has been + // persisted to object storage. + ROLLOVER_RETURN_ERROR = 3; + } + + // The size the WAL buffer should be limited to. Once the buffer gets to + // this size it will drop old segments to remain below this size, but + // still try to hold as much in memory as possible while remaining + // below this threshold + uint64 bufferSize = 1; + + // WAL segments become read only after crossing over this size. Which means + // that segments will always be >= this size. When old segments are + // dropped from of memory, at least this much space will be freed from + // the buffer. + uint64 segmentSize = 2; + + // What should happen if a write comes in that would exceed the WAL buffer + // size and the oldest segment that could be dropped hasn't yet been + // persisted to object storage. If the oldest segment has been + // persisted, then it will be dropped from the buffer so that new writes + // can be accepted. This option is only for defining the behavior of what + // happens if that segment hasn't been persisted. If set to return an + // error, new writes will be rejected until the oldest segment has been + // persisted so that it can be cleared from memory. Alternatively, this + // can be set so that old segments are dropped even if they haven't been + // persisted. This setting is also useful for cases where persistence + // isn't being used and this is only for in-memory buffering. + Rollover bufferRollover = 3; + + // If set to true, buffer segments will be written to object storage. + bool persistSegments = 4; + + // If set, segments will be rolled over after this period of time even + // if they haven't hit the size threshold. This allows them to be written + // out to object storage as they must be immutable first. + google.protobuf.Duration closeSegmentAfter = 5; +} + +message MutableBufferConfig { + message PartitionDropOrder { + message ColumnSort { + string columnName = 1; + ColumnType columnType = 2; + Aggregate columnValue = 3; + } + + // Sort partitions by this order. Last will be dropped first. + Order order = 1; + + // Configure sort key + oneof sort { + // The last time the partition received a write. + google.protobuf.Empty lastWriteTime = 2; + + // When the partition was opened in the mutable buffer. + google.protobuf.Empty createdAtTime = 3; + + // A column name, its expected type, and whether to use the min or max + // value. The ColumnType is necessary because the column can appear in + // any number of tables and be of a different type. This specifies that + // when sorting partitions, only columns with the given name and type + // should be used for the purposes of determining the partition order. If a + // partition doesn't have the given column in any way, the partition will + // appear at the beginning of the list with a null value where all + // partitions having null for that value will then be + // sorted by created_at_time desc. So if none of the partitions in the + // mutable buffer had this column with this type, then the partition + // that was created first would appear last in the list and thus be the + // first up to be dropped. + ColumnSort column = 4; + } + } + // The size the mutable buffer should be limited to. Once the buffer gets + // to this size it will drop partitions in the given order. If unable + // to drop partitions (because of later rules in this config) it will + // reject writes until it is able to drop partitions. + uint64 bufferSize = 1; + + // If set, the mutable buffer will not drop partitions that have chunks + // that have not yet been persisted. Thus it will reject writes if it + // is over size and is unable to drop partitions. The default is to + // drop partitions in the sort order, regardless of whether they have + // unpersisted chunks or not. The WAL Buffer can be used to ensure + // persistence, but this may cause longer recovery times. + bool rejectIfNotPersisted = 2; + + // Configure order to drop partitions in + PartitionDropOrder partitionDropOrder = 3; + + // Attempt to persist partitions after they haven't received a write for + // this number of seconds. If not set, partitions won't be + // automatically persisted. + uint32 persistAfterColdSeconds = 4; +} + +message DatabaseRules { + // The unencoded name of the database + string name = 1; + + // Template that generates a partition key for each row inserted into the database + PartitionTemplate partitionTemplate = 2; + + // Synchronous replication configuration for this database + ReplicationConfig replicationConfig = 3; + + // Asynchronous pull-based subscription configuration for this database + SubscriptionConfig subscriptionConfig = 4; + + // Query configuration for this database + QueryConfig queryConfig = 5; + + // WAL configuration for this database + WalBufferConfig walBufferConfig = 6; + + // Mutable buffer configuration for this database + MutableBufferConfig mutableBufferConfig = 7; +} \ No newline at end of file diff --git a/generated_types/protos/influxdata/iox/management/v1/service.proto b/generated_types/protos/influxdata/iox/management/v1/service.proto new file mode 100644 index 0000000000..af0392f6fe --- /dev/null +++ b/generated_types/protos/influxdata/iox/management/v1/service.proto @@ -0,0 +1,43 @@ +syntax = "proto3"; +package influxdata.iox.management.v1; + +import "google/protobuf/empty.proto"; +import "influxdata/iox/management/v1/database_rules.proto"; + +service Management { + rpc Ping(google.protobuf.Empty) returns (google.protobuf.Empty); + + rpc GetWriterId(google.protobuf.Empty) returns (GetWriterIdResponse); + + rpc UpdateWriterId(UpdateWriterIdRequest) returns (google.protobuf.Empty); + + rpc ListDatabases(google.protobuf.Empty) returns (ListDatabasesResponse); + + rpc GetDatabase(GetDatabaseRequest) returns (GetDatabaseResponse); + + rpc CreateDatabase(CreateDatabaseRequest) returns (google.protobuf.Empty); +} + +message GetWriterIdResponse { + uint32 id = 1; +} + +message UpdateWriterIdRequest { + uint32 id = 1; +} + +message ListDatabasesResponse { + repeated string names = 1; +} + +message GetDatabaseRequest { + string name = 1; +} + +message GetDatabaseResponse { + DatabaseRules rules = 1; +} + +message CreateDatabaseRequest { + DatabaseRules rules = 1; +} diff --git a/generated_types/predicate.proto b/generated_types/protos/influxdata/platform/storage/predicate.proto similarity index 100% rename from generated_types/predicate.proto rename to generated_types/protos/influxdata/platform/storage/predicate.proto diff --git a/generated_types/service.proto b/generated_types/protos/influxdata/platform/storage/service.proto similarity index 93% rename from generated_types/service.proto rename to generated_types/protos/influxdata/platform/storage/service.proto index 3ca768800a..3ed51d5bc5 100644 --- a/generated_types/service.proto +++ b/generated_types/protos/influxdata/platform/storage/service.proto @@ -8,9 +8,8 @@ syntax = "proto3"; package influxdata.platform.storage; import "google/protobuf/empty.proto"; -import "storage_common.proto"; -import "storage_common_idpe.proto"; - +import "influxdata/platform/storage/storage_common.proto"; +import "com/github/influxdata/idpe/storage/read/storage_common_idpe.proto"; service Storage { // ReadFilter performs a filter operation at storage diff --git a/generated_types/source.proto b/generated_types/protos/influxdata/platform/storage/source.proto similarity index 100% rename from generated_types/source.proto rename to generated_types/protos/influxdata/platform/storage/source.proto diff --git a/generated_types/storage_common.proto b/generated_types/protos/influxdata/platform/storage/storage_common.proto similarity index 99% rename from generated_types/storage_common.proto rename to generated_types/protos/influxdata/platform/storage/storage_common.proto index 0a1c16e83d..fce3153ec6 100644 --- a/generated_types/storage_common.proto +++ b/generated_types/protos/influxdata/platform/storage/storage_common.proto @@ -8,7 +8,7 @@ syntax = "proto3"; package influxdata.platform.storage; import "google/protobuf/any.proto"; -import "predicate.proto"; +import "influxdata/platform/storage/predicate.proto"; message ReadFilterRequest { diff --git a/generated_types/test.proto b/generated_types/protos/influxdata/platform/storage/test.proto similarity index 100% rename from generated_types/test.proto rename to generated_types/protos/influxdata/platform/storage/test.proto diff --git a/generated_types/wal.fbs b/generated_types/protos/wal.fbs similarity index 100% rename from generated_types/wal.fbs rename to generated_types/protos/wal.fbs diff --git a/generated_types/src/lib.rs b/generated_types/src/lib.rs index ab8649d83e..487164cad1 100644 --- a/generated_types/src/lib.rs +++ b/generated_types/src/lib.rs @@ -9,21 +9,55 @@ clippy::clone_on_ref_ptr )] -include!(concat!(env!("OUT_DIR"), "/influxdata.platform.storage.rs")); -include!(concat!( - env!("OUT_DIR"), - "/com.github.influxdata.idpe.storage.read.rs" -)); -include!(concat!(env!("OUT_DIR"), "/wal_generated.rs")); +mod pb { + pub mod influxdata { + pub mod platform { + pub mod storage { + include!(concat!(env!("OUT_DIR"), "/influxdata.platform.storage.rs")); -// Can't implement `Default` because `prost::Message` implements `Default` -impl TimestampRange { - pub fn max() -> Self { - TimestampRange { - start: std::i64::MIN, - end: std::i64::MAX, + // Can't implement `Default` because `prost::Message` implements `Default` + impl TimestampRange { + pub fn max() -> Self { + TimestampRange { + start: std::i64::MIN, + end: std::i64::MAX, + } + } + } + } + } + + pub mod iox { + pub mod management { + pub mod v1 { + include!(concat!(env!("OUT_DIR"), "/influxdata.iox.management.v1.rs")); + } + } + } + } + + pub mod com { + pub mod github { + pub mod influxdata { + pub mod idpe { + pub mod storage { + pub mod read { + include!(concat!( + env!("OUT_DIR"), + "/com.github.influxdata.idpe.storage.read.rs" + )); + } + } + } + } } } } +include!(concat!(env!("OUT_DIR"), "/wal_generated.rs")); + +pub use pb::com::github::influxdata::idpe::storage::read::*; +pub use pb::influxdata::platform::storage::*; + pub use google_types as google; +pub use pb::influxdata; From 43371a344caec3042186abe09a6e492e72521d1d Mon Sep 17 00:00:00 2001 From: Raphael Taylor-Davies Date: Mon, 22 Feb 2021 19:38:42 +0000 Subject: [PATCH 02/22] chore: documentation fixes --- data_types/src/database_rules.rs | 6 +++--- .../influxdata/iox/management/v1/database_rules.proto | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/data_types/src/database_rules.rs b/data_types/src/database_rules.rs index 14983ecc6c..7bfa430196 100644 --- a/data_types/src/database_rules.rs +++ b/data_types/src/database_rules.rs @@ -63,7 +63,7 @@ pub struct DatabaseRules { /// If set to `true`, this server should answer queries from one or more of /// of its local write buffer and any read-only partitions that it knows /// about. In this case, results will be merged with any others from the - /// remote goups or read only partitions. + /// remote goups or read-only partitions. #[serde(default)] pub query_local: bool, /// Set `primary_query_group` to a host group if remote servers should be @@ -254,7 +254,7 @@ pub struct WalBufferConfig { /// still try to hold as much in memory as possible while remaining /// below this threshold pub buffer_size: u64, - /// WAL segments become read only after crossing over this size. Which means + /// WAL segments become read-only after crossing over this size. Which means /// that segments will always be >= this size. When old segments are /// dropped from of memory, at least this much space will be freed from /// the buffer. @@ -285,7 +285,7 @@ pub struct WalBufferConfig { #[derive(Debug, Clone, Serialize, Deserialize, Eq, PartialEq, Copy)] pub enum WalBufferRollover { /// Drop the old segment even though it hasn't been persisted. This part of - /// the WAl will be lost on this server. + /// the WAL will be lost on this server. DropOldSegment, /// Drop the incoming write and fail silently. This favors making sure that /// older WAL data will be backed up. diff --git a/generated_types/protos/influxdata/iox/management/v1/database_rules.proto b/generated_types/protos/influxdata/iox/management/v1/database_rules.proto index 4e9a780a2b..f0d2d7d0ee 100644 --- a/generated_types/protos/influxdata/iox/management/v1/database_rules.proto +++ b/generated_types/protos/influxdata/iox/management/v1/database_rules.proto @@ -32,7 +32,7 @@ message PartitionTemplate { } message Matcher { - // Restrict selection to a specific table or table's specified by a regex + // Restrict selection to a specific table or tables specified by a regex oneof tableMatcher { string table = 1; string regex = 2; @@ -91,7 +91,7 @@ message QueryConfig { // If set to `true`, this server should answer queries from one or more of // of its local write buffer and any read-only partitions that it knows // about. In this case, results will be merged with any others from the - // remote goups or read only partitions. + // remote goups or read-only partitions. bool queryLocal = 1; // Set `primary` to a host group if remote servers should be @@ -123,7 +123,7 @@ message WalBufferConfig { ROLLOVER_UNKNOWN = 0; // Drop the old segment even though it hasn't been persisted. This part of - // the WAl will be lost on this server. + // the WAL will be lost on this server. ROLLOVER_DROP_OLD_SEGMENT = 1; // Drop the incoming write and fail silently. This favors making sure that @@ -142,7 +142,7 @@ message WalBufferConfig { // below this threshold uint64 bufferSize = 1; - // WAL segments become read only after crossing over this size. Which means + // WAL segments become read-only after crossing over this size. Which means // that segments will always be >= this size. When old segments are // dropped from of memory, at least this much space will be freed from // the buffer. From 1294d28f56ce8385758632cd4792973ff856307a Mon Sep 17 00:00:00 2001 From: Raphael Taylor-Davies Date: Tue, 23 Feb 2021 10:55:09 +0000 Subject: [PATCH 03/22] refactor: fix style errors --- generated_types/build.rs | 4 +- .../idpe/storage/read}/source.proto | 0 .../iox/management/v1/base_types.proto | 7 ++- .../iox/management/v1/database_rules.proto | 58 +++++++++---------- .../iox/management/v1/service.proto | 20 ++++--- .../influxdata/platform/storage/service.proto | 2 +- .../storage}/storage_common_idpe.proto | 0 7 files changed, 49 insertions(+), 42 deletions(-) rename generated_types/protos/{influxdata/platform/storage => com/github/influxdata/idpe/storage/read}/source.proto (100%) rename generated_types/protos/{com/github/influxdata/idpe/storage/read => influxdata/platform/storage}/storage_common_idpe.proto (100%) diff --git a/generated_types/build.rs b/generated_types/build.rs index b58e708cd3..b632f70123 100644 --- a/generated_types/build.rs +++ b/generated_types/build.rs @@ -34,8 +34,8 @@ fn generate_grpc_types(root: &Path) -> Result<()> { storage_path.join("predicate.proto"), storage_path.join("storage_common.proto"), storage_path.join("service.proto"), - storage_path.join("source.proto"), - idpe_path.join("storage_common_idpe.proto"), + storage_path.join("storage_common_idpe.proto"), + idpe_path.join("source.proto"), management_path.join("base_types.proto"), management_path.join("database_rules.proto"), management_path.join("service.proto"), diff --git a/generated_types/protos/influxdata/platform/storage/source.proto b/generated_types/protos/com/github/influxdata/idpe/storage/read/source.proto similarity index 100% rename from generated_types/protos/influxdata/platform/storage/source.proto rename to generated_types/protos/com/github/influxdata/idpe/storage/read/source.proto diff --git a/generated_types/protos/influxdata/iox/management/v1/base_types.proto b/generated_types/protos/influxdata/iox/management/v1/base_types.proto index 2618f44290..0b7cd130a2 100644 --- a/generated_types/protos/influxdata/iox/management/v1/base_types.proto +++ b/generated_types/protos/influxdata/iox/management/v1/base_types.proto @@ -2,19 +2,19 @@ syntax = "proto3"; package influxdata.iox.management.v1; enum Order { - ORDER_UNKNOWN = 0; + ORDER_UNSPECIFIED = 0; ORDER_ASC = 1; ORDER_DESC = 2; } enum Aggregate { - AGGREGATE_UNKNOWN = 0; + AGGREGATE_UNSPECIFIED = 0; AGGREGATE_MIN = 1; AGGREGATE_MAX = 2; } enum ColumnType { - COLUMN_TYPE_UNKNOWN = 0; + COLUMN_TYPE_UNSPECIFIED = 0; COLUMN_TYPE_I64 = 1; COLUMN_TYPE_U64 = 2; COLUMN_TYPE_F64 = 3; @@ -25,6 +25,7 @@ enum ColumnType { message HostGroup { string id = 1; + // connection strings for remote hosts. repeated string hosts = 2; } diff --git a/generated_types/protos/influxdata/iox/management/v1/database_rules.proto b/generated_types/protos/influxdata/iox/management/v1/database_rules.proto index f0d2d7d0ee..e09f3e18f7 100644 --- a/generated_types/protos/influxdata/iox/management/v1/database_rules.proto +++ b/generated_types/protos/influxdata/iox/management/v1/database_rules.proto @@ -24,7 +24,7 @@ message PartitionTemplate { string column = 2; string time = 3; ColumnFormat regex = 4; - ColumnFormat strfTime = 5; + ColumnFormat strf_time = 5; } } @@ -33,7 +33,7 @@ message PartitionTemplate { message Matcher { // Restrict selection to a specific table or tables specified by a regex - oneof tableMatcher { + oneof table_matcher { string table = 1; string regex = 2; } @@ -43,7 +43,7 @@ message Matcher { message ReplicationConfig { message Replication { - string hostGroupId = 1; + string host_group_id = 1; } // The set of host groups that data should be replicated to. Which host a @@ -62,19 +62,19 @@ message ReplicationConfig { // is returned. This can be overridden on a per request basis. // Replication will continue to write to the other host groups in the // background. - uint32 replicationCount = 2; + uint32 replication_count = 2; // How long the replication queue can get before either rejecting writes or // dropping missed writes. The queue is kept in memory on a // per-database basis. A queue size of zero means it will only try to // replicate synchronously and drop any failures. - uint64 replicationQueueMaxSize = 3; + uint64 replication_queue_max_size = 3; } message SubscriptionConfig { message Subscription { string name = 1; - string hostGroupId = 2; + string host_group_id = 2; Matcher matcher = 3; } @@ -92,7 +92,7 @@ message QueryConfig { // of its local write buffer and any read-only partitions that it knows // about. In this case, results will be merged with any others from the // remote goups or read-only partitions. - bool queryLocal = 1; + bool query_local = 1; // Set `primary` to a host group if remote servers should be // issued queries for this database. All hosts in the group should be @@ -115,12 +115,12 @@ message QueryConfig { // collection of partitions and then telling it to also pull // data from the replication servers (writes that haven't been snapshotted // into a partition). - repeated string readOnlyPartitions = 4; + repeated string read_only_partitions = 4; } message WalBufferConfig { enum Rollover { - ROLLOVER_UNKNOWN = 0; + ROLLOVER_UNSPECIFIED = 0; // Drop the old segment even though it hasn't been persisted. This part of // the WAL will be lost on this server. @@ -140,13 +140,13 @@ message WalBufferConfig { // this size it will drop old segments to remain below this size, but // still try to hold as much in memory as possible while remaining // below this threshold - uint64 bufferSize = 1; + uint64 buffer_size = 1; // WAL segments become read-only after crossing over this size. Which means // that segments will always be >= this size. When old segments are // dropped from of memory, at least this much space will be freed from // the buffer. - uint64 segmentSize = 2; + uint64 segment_size = 2; // What should happen if a write comes in that would exceed the WAL buffer // size and the oldest segment that could be dropped hasn't yet been @@ -159,23 +159,23 @@ message WalBufferConfig { // can be set so that old segments are dropped even if they haven't been // persisted. This setting is also useful for cases where persistence // isn't being used and this is only for in-memory buffering. - Rollover bufferRollover = 3; + Rollover buffer_rollover = 3; // If set to true, buffer segments will be written to object storage. - bool persistSegments = 4; + bool persist_segments = 4; // If set, segments will be rolled over after this period of time even // if they haven't hit the size threshold. This allows them to be written // out to object storage as they must be immutable first. - google.protobuf.Duration closeSegmentAfter = 5; + google.protobuf.Duration close_segment_after = 5; } message MutableBufferConfig { message PartitionDropOrder { message ColumnSort { - string columnName = 1; - ColumnType columnType = 2; - Aggregate columnValue = 3; + string column_name = 1; + ColumnType column_type = 2; + Aggregate column_value = 3; } // Sort partitions by this order. Last will be dropped first. @@ -184,10 +184,10 @@ message MutableBufferConfig { // Configure sort key oneof sort { // The last time the partition received a write. - google.protobuf.Empty lastWriteTime = 2; + google.protobuf.Empty last_write_time = 2; // When the partition was opened in the mutable buffer. - google.protobuf.Empty createdAtTime = 3; + google.protobuf.Empty created_at_time = 3; // A column name, its expected type, and whether to use the min or max // value. The ColumnType is necessary because the column can appear in @@ -208,7 +208,7 @@ message MutableBufferConfig { // to this size it will drop partitions in the given order. If unable // to drop partitions (because of later rules in this config) it will // reject writes until it is able to drop partitions. - uint64 bufferSize = 1; + uint64 buffer_size = 1; // If set, the mutable buffer will not drop partitions that have chunks // that have not yet been persisted. Thus it will reject writes if it @@ -216,15 +216,15 @@ message MutableBufferConfig { // drop partitions in the sort order, regardless of whether they have // unpersisted chunks or not. The WAL Buffer can be used to ensure // persistence, but this may cause longer recovery times. - bool rejectIfNotPersisted = 2; + bool reject_if_not_persisted = 2; // Configure order to drop partitions in - PartitionDropOrder partitionDropOrder = 3; + PartitionDropOrder partition_drop_order = 3; // Attempt to persist partitions after they haven't received a write for // this number of seconds. If not set, partitions won't be // automatically persisted. - uint32 persistAfterColdSeconds = 4; + uint32 persist_after_cold_seconds = 4; } message DatabaseRules { @@ -232,20 +232,20 @@ message DatabaseRules { string name = 1; // Template that generates a partition key for each row inserted into the database - PartitionTemplate partitionTemplate = 2; + PartitionTemplate partition_template = 2; // Synchronous replication configuration for this database - ReplicationConfig replicationConfig = 3; + ReplicationConfig replication_config = 3; // Asynchronous pull-based subscription configuration for this database - SubscriptionConfig subscriptionConfig = 4; + SubscriptionConfig subscription_config = 4; // Query configuration for this database - QueryConfig queryConfig = 5; + QueryConfig query_config = 5; // WAL configuration for this database - WalBufferConfig walBufferConfig = 6; + WalBufferConfig wal_buffer_config = 6; // Mutable buffer configuration for this database - MutableBufferConfig mutableBufferConfig = 7; + MutableBufferConfig mutable_buffer_config = 7; } \ No newline at end of file diff --git a/generated_types/protos/influxdata/iox/management/v1/service.proto b/generated_types/protos/influxdata/iox/management/v1/service.proto index af0392f6fe..73735431c1 100644 --- a/generated_types/protos/influxdata/iox/management/v1/service.proto +++ b/generated_types/protos/influxdata/iox/management/v1/service.proto @@ -4,20 +4,20 @@ package influxdata.iox.management.v1; import "google/protobuf/empty.proto"; import "influxdata/iox/management/v1/database_rules.proto"; -service Management { - rpc Ping(google.protobuf.Empty) returns (google.protobuf.Empty); +service ManagementService { + rpc GetWriterId(GetWriterIdRequest) returns (GetWriterIdResponse); - rpc GetWriterId(google.protobuf.Empty) returns (GetWriterIdResponse); + rpc UpdateWriterId(UpdateWriterIdRequest) returns (UpdateWriterIdResponse); - rpc UpdateWriterId(UpdateWriterIdRequest) returns (google.protobuf.Empty); - - rpc ListDatabases(google.protobuf.Empty) returns (ListDatabasesResponse); + rpc ListDatabases(ListDatabasesRequest) returns (ListDatabasesResponse); rpc GetDatabase(GetDatabaseRequest) returns (GetDatabaseResponse); - rpc CreateDatabase(CreateDatabaseRequest) returns (google.protobuf.Empty); + rpc CreateDatabase(CreateDatabaseRequest) returns (CreateDatabaseResponse); } +message GetWriterIdRequest {} + message GetWriterIdResponse { uint32 id = 1; } @@ -26,6 +26,10 @@ message UpdateWriterIdRequest { uint32 id = 1; } +message UpdateWriterIdResponse {} + +message ListDatabasesRequest {} + message ListDatabasesResponse { repeated string names = 1; } @@ -41,3 +45,5 @@ message GetDatabaseResponse { message CreateDatabaseRequest { DatabaseRules rules = 1; } + +message CreateDatabaseResponse {} diff --git a/generated_types/protos/influxdata/platform/storage/service.proto b/generated_types/protos/influxdata/platform/storage/service.proto index 3ed51d5bc5..9d2e78206c 100644 --- a/generated_types/protos/influxdata/platform/storage/service.proto +++ b/generated_types/protos/influxdata/platform/storage/service.proto @@ -9,7 +9,7 @@ package influxdata.platform.storage; import "google/protobuf/empty.proto"; import "influxdata/platform/storage/storage_common.proto"; -import "com/github/influxdata/idpe/storage/read/storage_common_idpe.proto"; +import "influxdata/platform/storage/storage_common_idpe.proto"; service Storage { // ReadFilter performs a filter operation at storage diff --git a/generated_types/protos/com/github/influxdata/idpe/storage/read/storage_common_idpe.proto b/generated_types/protos/influxdata/platform/storage/storage_common_idpe.proto similarity index 100% rename from generated_types/protos/com/github/influxdata/idpe/storage/read/storage_common_idpe.proto rename to generated_types/protos/influxdata/platform/storage/storage_common_idpe.proto From 0fc711c9dfb17d916c159dc90ba23cc80a172ef5 Mon Sep 17 00:00:00 2001 From: Raphael Taylor-Davies Date: Tue, 23 Feb 2021 16:36:11 +0000 Subject: [PATCH 04/22] fix: don't use default Matcher.table_matcher "variant" --- .../influxdata/iox/management/v1/base_types.proto | 1 - .../influxdata/iox/management/v1/database_rules.proto | 11 ++++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/generated_types/protos/influxdata/iox/management/v1/base_types.proto b/generated_types/protos/influxdata/iox/management/v1/base_types.proto index 0b7cd130a2..90398b778a 100644 --- a/generated_types/protos/influxdata/iox/management/v1/base_types.proto +++ b/generated_types/protos/influxdata/iox/management/v1/base_types.proto @@ -28,4 +28,3 @@ message HostGroup { // connection strings for remote hosts. repeated string hosts = 2; } - diff --git a/generated_types/protos/influxdata/iox/management/v1/database_rules.proto b/generated_types/protos/influxdata/iox/management/v1/database_rules.proto index e09f3e18f7..5d669419a0 100644 --- a/generated_types/protos/influxdata/iox/management/v1/database_rules.proto +++ b/generated_types/protos/influxdata/iox/management/v1/database_rules.proto @@ -32,13 +32,14 @@ message PartitionTemplate { } message Matcher { + // A query predicate to filter rows + string predicate = 1; // Restrict selection to a specific table or tables specified by a regex oneof table_matcher { - string table = 1; - string regex = 2; + google.protobuf.Empty all = 2; + string table = 3; + string regex = 4; } - // A query predicate to filter rows - string predicate = 3; } message ReplicationConfig { @@ -248,4 +249,4 @@ message DatabaseRules { // Mutable buffer configuration for this database MutableBufferConfig mutable_buffer_config = 7; -} \ No newline at end of file +} From a7a77d9cd737f0e12c47050846adcbfc5a10725d Mon Sep 17 00:00:00 2001 From: Raphael Taylor-Davies Date: Tue, 23 Feb 2021 17:02:15 +0000 Subject: [PATCH 05/22] refactor: remove unused Replication message --- .../protos/influxdata/iox/management/v1/database_rules.proto | 4 ---- 1 file changed, 4 deletions(-) diff --git a/generated_types/protos/influxdata/iox/management/v1/database_rules.proto b/generated_types/protos/influxdata/iox/management/v1/database_rules.proto index 5d669419a0..df953f4224 100644 --- a/generated_types/protos/influxdata/iox/management/v1/database_rules.proto +++ b/generated_types/protos/influxdata/iox/management/v1/database_rules.proto @@ -43,10 +43,6 @@ message Matcher { } message ReplicationConfig { - message Replication { - string host_group_id = 1; - } - // The set of host groups that data should be replicated to. Which host a // write goes to within a host group is determined by consistent hashing of // the partition key. We'd use this to create a host group per From 4009a89239996ce6a447179695c0b9ae1b9ac170 Mon Sep 17 00:00:00 2001 From: Raphael Taylor-Davies Date: Tue, 23 Feb 2021 19:17:49 +0000 Subject: [PATCH 06/22] feat: add buf linting in CI --- .github/workflows/rust.yml | 12 +++++++++++- buf.yaml | 17 +++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 buf.yaml diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index a04455a1b9..b882ff9cc0 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -71,7 +71,7 @@ jobs: args: --workspace lints: - name: Lints + name: Rust Lints runs-on: ubuntu-latest container: image: quay.io/influxdb/rust:ci @@ -91,3 +91,13 @@ jobs: with: token: ${{ secrets.GITHUB_TOKEN }} args: --all-targets --workspace -- -D warnings + + protobuf: + name: Protobuf Lints + runs-on: ubuntu-latest + container: + image: bufbuild/buf + steps: + - uses: actions/checkout@v2 + - name: Lint IOx protobuf + run: buf lint diff --git a/buf.yaml b/buf.yaml new file mode 100644 index 0000000000..030255f9de --- /dev/null +++ b/buf.yaml @@ -0,0 +1,17 @@ +version: v1beta1 +build: + roots: + - generated_types/protos/ + excludes: + - generated_types/protos/com + - generated_types/protos/influxdata/platform + +lint: + use: + - DEFAULT + - STYLE_DEFAULT + +breaking: + use: + - WIRE + - WIRE_JSON From 5446f543d6a33a8ae377bcc3351f8713d3145998 Mon Sep 17 00:00:00 2001 From: Marko Mikulicic Date: Mon, 22 Feb 2021 10:06:49 +0100 Subject: [PATCH 07/22] feat: Implement /iox/api/v1/databases/:name/query?q= Partially implements #726 TODO in another PR: `format` parameter. This PR implements the pretty print format. --- src/influxdb_ioxd/http.rs | 99 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/src/influxdb_ioxd/http.rs b/src/influxdb_ioxd/http.rs index ee15b1dfbb..07adc1b96b 100644 --- a/src/influxdb_ioxd/http.rs +++ b/src/influxdb_ioxd/http.rs @@ -263,6 +263,7 @@ where .get("/iox/api/v1/databases", list_databases::) .put("/iox/api/v1/databases/:name", create_database::) .get("/iox/api/v1/databases/:name", get_database::) + .get("/iox/api/v1/databases/:name/query", query::) .get("/iox/api/v1/databases/:name/wal/meta", get_wal_meta::) .put("/iox/api/v1/id", set_writer::) .get("/iox/api/v1/id", get_writer::) @@ -455,6 +456,54 @@ async fn read( Ok(Response::new(Body::from(results.into_bytes()))) } +#[derive(Deserialize, Debug)] +/// Body of the request to the .../query endpoint +struct QueryInfo { + q: String, +} + +// TODO: figure out how to stream read results out rather than rendering the +// whole thing in mem +#[tracing::instrument(level = "debug")] +async fn query( + req: Request, +) -> Result, ApplicationError> { + let server = Arc::clone(&req.data::>>().expect("server state")); + + let query = req.uri().query().context(ExpectedQueryString {})?; + + let query_info: QueryInfo = serde_urlencoded::from_str(query).context(InvalidQueryString { + query_string: query, + })?; + + let db_name_str = req + .param("name") + .expect("db name must have been set") + .clone(); + let db_name = DatabaseName::new(&db_name_str).context(DatabaseNameError)?; + let db = server + .db(&db_name) + .await + .context(DatabaseNotFound { name: &db_name_str })?; + + let planner = SQLQueryPlanner::default(); + let executor = server.executor(); + + let physical_plan = planner + .query(db.as_ref(), &query_info.q, executor.as_ref()) + .await + .context(PlanningSQLQuery { query })?; + + let batches = collect(physical_plan) + .await + .map_err(|e| Box::new(e) as _) + .context(Query { db_name })?; + + let results = arrow::util::pretty::pretty_format_batches(&batches).unwrap(); + + Ok(Response::new(Body::from(results.into_bytes()))) +} + #[tracing::instrument(level = "debug")] async fn list_databases(req: Request) -> Result, ApplicationError> where @@ -833,6 +882,56 @@ mod tests { Ok(()) } + #[tokio::test] + async fn test_query() -> Result<()> { + let test_storage = Arc::new(AppServer::new( + ConnectionManagerImpl {}, + Arc::new(ObjectStore::new_in_memory(InMemory::new())), + )); + test_storage.set_id(1); + test_storage + .create_database("MyOrg_MyBucket", DatabaseRules::new()) + .await + .unwrap(); + let server_url = test_server(Arc::clone(&test_storage)); + + let client = Client::new(); + + let lp_data = "h2o_temperature,location=santa_monica,state=CA surface_degrees=65.2,bottom_degrees=50.4 1568756160"; + + // send write data + let bucket_name = "MyBucket"; + let org_name = "MyOrg"; + let response = client + .post(&format!( + "{}/api/v2/write?bucket={}&org={}", + server_url, bucket_name, org_name + )) + .body(lp_data) + .send() + .await; + + check_response("write", response, StatusCode::NO_CONTENT, "").await; + + // send query data + let response = client + .get(&format!( + "{}/iox/api/v1/databases/MyOrg_MyBucket/query?q={}", + server_url, "select%20*%20from%20h2o_temperature" + )) + .send() + .await; + + let res = "+----------------+--------------+-------+-----------------+------------+\n\ + | bottom_degrees | location | state | surface_degrees | time |\n\ + +----------------+--------------+-------+-----------------+------------+\n\ + | 50.4 | santa_monica | CA | 65.2 | 1568756160 |\n\ + +----------------+--------------+-------+-----------------+------------+\n"; + check_response("query", response, StatusCode::OK, res).await; + + Ok(()) + } + fn gzip_str(s: &str) -> Vec { use flate2::{write::GzEncoder, Compression}; use std::io::Write; From 12b768b8f1cf50f1dcdd7d99be3f0b21814034f7 Mon Sep 17 00:00:00 2001 From: Marko Mikulicic Date: Mon, 22 Feb 2021 18:40:40 +0100 Subject: [PATCH 08/22] fix: Escape empty string PathPart Empty directory names are silently ignored and can lead to very surprising effects such as directory layouts missing a level. This makes it hard to reason about directory structures. A sane object store path API should either disallow empty names or deal with them gracefully. Since we already have to escape file/directory names using the minimum common denominator valid character set for known cloud providers, it feels quite natural to treat this empty dir/file name problem as encoding problem. --- object_store/src/path/parts.rs | 44 +++++++++++++++++++++++++++++----- 1 file changed, 38 insertions(+), 6 deletions(-) diff --git a/object_store/src/path/parts.rs b/object_store/src/path/parts.rs index 5ce18dc1d6..3626b305b2 100644 --- a/object_store/src/path/parts.rs +++ b/object_store/src/path/parts.rs @@ -5,11 +5,17 @@ use super::DELIMITER; // percent_encode's API needs this as a byte const DELIMITER_BYTE: u8 = DELIMITER.as_bytes()[0]; +// special encoding of the empty string part. +// Using '%' is the safest character since it will always be used in the +// output of percent_encode no matter how we evolve the INVALID AsciiSet over +// time. +const EMPTY: &str = "%"; + /// The PathPart type exists to validate the directory/file names that form part /// of a path. /// -/// A PathPart instance is guaranteed to contain no `/` characters as it can -/// only be constructed by going through the `try_from` impl. +/// A PathPart instance is guaranteed to be non-empty and to contain no `/` +/// characters as it can only be constructed by going through the `from` impl. #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Debug, Default)] pub struct PathPart(pub(super) String); @@ -48,6 +54,12 @@ impl From<&str> for PathPart { // to be equal to `.` or `..` to prevent file system traversal shenanigans. "." => Self(String::from("%2E")), ".." => Self(String::from("%2E%2E")), + + // Every string except the empty string will be percent encoded. + // The empty string will be transformed into a sentinel value EMPTY + // which can safely be a prefix of an encoded value since it will be + // fully matched at decode time (see impl Display for PathPart). + "" => Self(String::from(EMPTY)), other => Self(percent_encode(other.as_bytes(), INVALID).to_string()), } } @@ -55,10 +67,13 @@ impl From<&str> for PathPart { impl std::fmt::Display for PathPart { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - percent_decode_str(&self.0) - .decode_utf8() - .expect("Valid UTF-8 that came from String") - .fmt(f) + match &self.0[..] { + EMPTY => "".fmt(f), + _ => percent_decode_str(&self.0) + .decode_utf8() + .expect("Valid UTF-8 that came from String") + .fmt(f), + } } } @@ -104,4 +119,21 @@ mod tests { assert_eq!(part, PathPart(String::from("%2E%2E"))); assert_eq!(part.to_string(), ".."); } + + #[test] + fn path_part_cant_be_empty() { + let part: PathPart = "".into(); + assert_eq!(part, PathPart(String::from(EMPTY))); + assert_eq!(part.to_string(), ""); + } + + #[test] + fn empty_is_safely_encoded() { + let part: PathPart = EMPTY.into(); + assert_eq!( + part, + PathPart(percent_encode(EMPTY.as_bytes(), INVALID).to_string()) + ); + assert_eq!(part.to_string(), EMPTY); + } } From 4e338cd7e8648b213d7061ed2357f678e300518d Mon Sep 17 00:00:00 2001 From: Marko Mikulicic Date: Wed, 24 Feb 2021 23:05:58 +0100 Subject: [PATCH 09/22] fix: Remove test for non-existing DNS record --- src/commands/config.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/commands/config.rs b/src/commands/config.rs index b12c632425..0840038004 100644 --- a/src/commands/config.rs +++ b/src/commands/config.rs @@ -301,15 +301,6 @@ mod tests { clap::ErrorKind::ValueValidation ); - assert_eq!( - Config::from_iter_safe(strip_server( - to_vec(&["cmd", "server", "--api-bind", "badhost.badtld:1234"]).into_iter(), - )) - .map_err(|e| e.kind) - .expect_err("must fail"), - clap::ErrorKind::ValueValidation - ); - Ok(()) } } From 8fb7651719fe7c4ddda6107639aa765e5a9da7ca Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Wed, 24 Feb 2021 18:11:22 -0500 Subject: [PATCH 10/22] feat: Port tag_values to the InfluxRPCPlanner (#859) * feat: Port tag_values to the InfluxRPCPlanner * refactor: merge imports * refactor: rename column_names to tag_column_names for clarity * fix: Update query/src/frontend/influxrpc.rs Co-authored-by: Carol (Nichols || Goulding) <193874+carols10cents@users.noreply.github.com> * refactor: use ensure! Co-authored-by: Carol (Nichols || Goulding) <193874+carols10cents@users.noreply.github.com> * refactor: less silly whitespace Co-authored-by: Carol (Nichols || Goulding) <193874+carols10cents@users.noreply.github.com> * fix: code review comments Co-authored-by: Carol (Nichols || Goulding) <193874+carols10cents@users.noreply.github.com> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --- mutable_buffer/src/chunk.rs | 164 +++++++- mutable_buffer/src/database.rs | 365 +----------------- mutable_buffer/src/table.rs | 46 +-- query/src/frontend/influxrpc.rs | 197 +++++++++- query/src/lib.rs | 24 +- query/src/test.rs | 70 +--- server/src/db.rs | 15 +- server/src/db/chunk.rs | 36 ++ server/src/query_tests/influxrpc.rs | 1 + .../src/query_tests/influxrpc/tag_values.rs | 253 ++++++++++++ src/influxdb_ioxd/rpc/storage/service.rs | 170 ++++---- 11 files changed, 774 insertions(+), 567 deletions(-) create mode 100644 server/src/query_tests/influxrpc/tag_values.rs diff --git a/mutable_buffer/src/chunk.rs b/mutable_buffer/src/chunk.rs index 8acbf63aac..119df751f3 100644 --- a/mutable_buffer/src/chunk.rs +++ b/mutable_buffer/src/chunk.rs @@ -24,9 +24,11 @@ use query::{ util::{make_range_expr, AndExprBuilder}, }; -use crate::dictionary::{Dictionary, Error as DictionaryError}; -use crate::table::Table; - +use crate::{ + column::Column, + dictionary::{Dictionary, Error as DictionaryError}, + table::Table, +}; use async_trait::async_trait; use snafu::{OptionExt, ResultExt, Snafu}; @@ -50,6 +52,12 @@ pub enum Error { source: crate::table::Error, }, + #[snafu(display("Error checking predicate in table '{}': {}", table_name, source))] + NamedTablePredicateCheck { + table_name: String, + source: crate::table::Error, + }, + #[snafu(display( "Unsupported predicate when mutable buffer table names. Found a general expression: {:?}", exprs @@ -85,12 +93,36 @@ pub enum Error { #[snafu(display("Attempt to write table batch without a name"))] TableWriteWithoutName, + #[snafu(display("Value ID {} not found in dictionary of chunk {}", value_id, chunk_id))] + InternalColumnValueIdNotFoundInDictionary { + value_id: u32, + chunk_id: u64, + source: DictionaryError, + }, + #[snafu(display("Column ID {} not found in dictionary of chunk {}", column_id, chunk))] ColumnIdNotFoundInDictionary { column_id: u32, chunk: u64, source: DictionaryError, }, + + #[snafu(display( + "Column name {} not found in dictionary of chunk {}", + column_name, + chunk_id + ))] + ColumnNameNotFoundInDictionary { + column_name: String, + chunk_id: u64, + source: DictionaryError, + }, + + #[snafu(display( + "Column '{}' is not a string tag column and thus can not list values", + column_name + ))] + UnsupportedColumnTypeForListingValues { column_name: String }, } pub type Result = std::result::Result; @@ -311,13 +343,7 @@ impl Chunk { return Ok(None); } - let table_name_id = - self.dictionary - .id(table_name) - .context(InternalTableNotFoundInDictionary { - table_name, - chunk_id: self.id(), - })?; + let table_name_id = self.table_name_id(table_name)?; let mut chunk_column_ids = BTreeSet::new(); @@ -352,6 +378,115 @@ impl Chunk { Ok(Some(column_names)) } + /// Return the id of the table in the chunk's dictionary + fn table_name_id(&self, table_name: &str) -> Result { + self.dictionary + .id(table_name) + .context(InternalTableNotFoundInDictionary { + table_name, + chunk_id: self.id(), + }) + } + + /// Returns the strings of the specified Tag column that satisfy + /// the predicate, if they can be determined entirely using metadata. + /// + /// If the predicate cannot be evaluated entirely with metadata, + /// return `Ok(None)`. + pub fn tag_column_values( + &self, + table_name: &str, + column_name: &str, + chunk_predicate: &ChunkPredicate, + ) -> Result>> { + // No support for general purpose expressions + if !chunk_predicate.chunk_exprs.is_empty() { + return Ok(None); + } + let chunk_id = self.id(); + + let table_name_id = self.table_name_id(table_name)?; + + // Is this table even in the chunk? + let table = self + .tables + .get(&table_name_id) + .context(NamedTableNotFoundInChunk { + table_name, + chunk_id, + })?; + + // See if we can rule out the table entire on metadata + let could_match = table + .could_match_predicate(chunk_predicate) + .context(NamedTablePredicateCheck { table_name })?; + + if !could_match { + // No columns could match, return empty set + return Ok(Default::default()); + } + + let column_id = + self.dictionary + .lookup_value(column_name) + .context(ColumnNameNotFoundInDictionary { + column_name, + chunk_id, + })?; + + let column = table + .column(column_id) + .context(NamedTableError { table_name })?; + + if let Column::Tag(column, _) = column { + // if we have a timestamp predicate, find all values + // where the timestamp is within range. Otherwise take + // all values. + + // Collect matching ids into BTreeSet to deduplicate on + // ids *before* looking up Strings + let column_value_ids: BTreeSet = match chunk_predicate.range { + None => { + // take all non-null values + column.iter().filter_map(|&s| s).collect() + } + Some(range) => { + // filter out all values that don't match the timestmap + let time_column = table + .column_i64(chunk_predicate.time_column_id) + .context(NamedTableError { table_name })?; + + column + .iter() + .zip(time_column.iter()) + .filter_map(|(&column_value_id, ×tamp_value)| { + if range.contains_opt(timestamp_value) { + column_value_id + } else { + None + } + }) + .collect() + } + }; + + // convert all the (deduplicated) ids to Strings + let column_values = column_value_ids + .into_iter() + .map(|value_id| { + let value = self.dictionary.lookup_id(value_id).context( + InternalColumnValueIdNotFoundInDictionary { value_id, chunk_id }, + )?; + Ok(value.to_string()) + }) + .collect::>>()?; + + Ok(Some(column_values)) + } else { + UnsupportedColumnTypeForListingValues { column_name }.fail() + } + } + /// Translates `predicate` into per-chunk ids that can be /// directly evaluated against tables in this chunk pub fn compile_predicate(&self, predicate: &Predicate) -> Result { @@ -612,6 +747,15 @@ impl query::PartitionChunk for Chunk { ) -> Result, Self::Error> { unimplemented!("This function is slated for removal") } + + async fn column_values( + &self, + _table_name: &str, + _column_name: &str, + _predicate: &Predicate, + ) -> Result, Self::Error> { + unimplemented!("This function is slated for removal") + } } /// Used to figure out if we know how to deal with this kind of diff --git a/mutable_buffer/src/database.rs b/mutable_buffer/src/database.rs index 4c57fc33c5..6641faeb9f 100644 --- a/mutable_buffer/src/database.rs +++ b/mutable_buffer/src/database.rs @@ -3,14 +3,14 @@ use data_types::{ database_rules::{PartitionSort, PartitionSortRules}, }; use generated_types::wal; +use query::group_by::Aggregate; use query::group_by::GroupByAndAggregate; use query::group_by::WindowDuration; use query::{ - exec::{stringset::StringSet, SeriesSetPlan, SeriesSetPlans}, + exec::{SeriesSetPlan, SeriesSetPlans}, predicate::Predicate, Database, }; -use query::{group_by::Aggregate, plan::stringset::StringSetPlan}; use crate::column::Column; use crate::table::Table; @@ -19,10 +19,10 @@ use crate::{ partition::Partition, }; -use std::collections::{BTreeSet, HashMap, HashSet}; +use std::collections::{HashMap, HashSet}; use std::sync::Arc; -use arrow_deps::datafusion::{error::DataFusionError, logical_plan::LogicalPlan}; +use arrow_deps::datafusion::error::DataFusionError; use crate::dictionary::Error as DictionaryError; @@ -46,30 +46,6 @@ pub enum Error { source: DictionaryError, }, - #[snafu(display( - "Column name {} not found in dictionary of chunk {}", - column_name, - chunk - ))] - ColumnNameNotFoundInDictionary { - column_name: String, - chunk: u64, - source: DictionaryError, - }, - - #[snafu(display("Value ID {} not found in dictionary of chunk {}", value_id, chunk))] - ColumnValueIdNotFoundInDictionary { - value_id: u32, - chunk: u64, - source: DictionaryError, - }, - - #[snafu(display( - "Column '{}' is not a tag column and thus can not list values", - column_name - ))] - UnsupportedColumnTypeForListingValues { column_name: String }, - #[snafu(display("id conversion error"))] IdConversionError { source: std::num::TryFromIntError }, @@ -254,27 +230,6 @@ impl Database for MutableBufferDb { Ok(()) } - /// return all column values in this database, while applying optional - /// predicates - async fn column_values( - &self, - column_name: &str, - predicate: Predicate, - ) -> Result { - let has_exprs = predicate.has_exprs(); - let mut filter = ChunkTableFilter::new(predicate); - - if has_exprs { - let mut visitor = ValuePredVisitor::new(column_name); - self.accept(&mut filter, &mut visitor)?; - Ok(visitor.plans.into()) - } else { - let mut visitor = ValueVisitor::new(column_name); - self.accept(&mut filter, &mut visitor)?; - Ok(visitor.column_values.into()) - } - } - async fn query_series(&self, predicate: Predicate) -> Result { let mut filter = ChunkTableFilter::new(predicate); let mut visitor = SeriesVisitor::new(); @@ -569,152 +524,6 @@ impl ChunkTableFilter { } } -/// return all values in the `column_name` column -/// in this database, while applying the timestamp range -/// -/// Potential optimizations: Run this in parallel (in different -/// futures) for each chunk / table, rather than a single one -/// -- but that will require building up parallel hash tables. -struct ValueVisitor<'a> { - column_name: &'a str, - // what column id we are looking for - column_id: Option, - chunk_value_ids: BTreeSet, - column_values: StringSet, -} - -impl<'a> ValueVisitor<'a> { - fn new(column_name: &'a str) -> Self { - Self { - column_name, - column_id: None, - column_values: StringSet::new(), - chunk_value_ids: BTreeSet::new(), - } - } -} - -impl<'a> Visitor for ValueVisitor<'a> { - fn pre_visit_chunk(&mut self, chunk: &Chunk) -> Result<()> { - self.chunk_value_ids.clear(); - - self.column_id = Some(chunk.dictionary.lookup_value(self.column_name).context( - ColumnNameNotFoundInDictionary { - column_name: self.column_name, - chunk: chunk.id, - }, - )?); - - Ok(()) - } - - fn visit_column( - &mut self, - table: &Table, - column_id: u32, - column: &Column, - filter: &mut ChunkTableFilter, - ) -> Result<()> { - if Some(column_id) != self.column_id { - return Ok(()); - } - - match column { - Column::Tag(column, _) => { - // if we have a timestamp prediate, find all values - // where the timestamp is within range. Otherwise take - // all values. - let chunk_predicate = filter.chunk_predicate(); - match chunk_predicate.range { - None => { - // take all non-null values - column.iter().filter_map(|&s| s).for_each(|value_id| { - self.chunk_value_ids.insert(value_id); - }); - } - Some(range) => { - // filter out all values that don't match the timestmap - let time_column = table.column_i64(chunk_predicate.time_column_id)?; - - column - .iter() - .zip(time_column.iter()) - .filter_map(|(&column_value_id, ×tamp_value)| { - if range.contains_opt(timestamp_value) { - column_value_id - } else { - None - } - }) - .for_each(|value_id| { - self.chunk_value_ids.insert(value_id); - }); - } - } - Ok(()) - } - _ => UnsupportedColumnTypeForListingValues { - column_name: self.column_name, - } - .fail(), - } - } - - fn post_visit_chunk(&mut self, chunk: &Chunk) -> Result<()> { - // convert all the chunk's column_ids to Strings - for &value_id in &self.chunk_value_ids { - let value = chunk.dictionary.lookup_id(value_id).context( - ColumnValueIdNotFoundInDictionary { - value_id, - chunk: chunk.id, - }, - )?; - - if !self.column_values.contains(value) { - self.column_values.insert(value.to_string()); - } - } - Ok(()) - } -} - -/// return all column values for the specified column in this -/// database, while applying the timestamp range and predicate -struct ValuePredVisitor<'a> { - column_name: &'a str, - plans: Vec, -} - -impl<'a> ValuePredVisitor<'a> { - fn new(column_name: &'a str) -> Self { - Self { - column_name, - plans: Vec::new(), - } - } -} - -impl<'a> Visitor for ValuePredVisitor<'a> { - // TODO try and rule out entire tables based on the same critera - // as explained on NamePredVisitor - fn pre_visit_table( - &mut self, - table: &Table, - chunk: &Chunk, - filter: &mut ChunkTableFilter, - ) -> Result<()> { - // skip table entirely if there are no rows that fall in the timestamp - if table.could_match_predicate(filter.chunk_predicate())? { - self.plans.push(table.tag_values_plan( - self.column_name, - filter.chunk_predicate(), - chunk, - )?); - } - Ok(()) - } -} - /// Return DataFusion plans to calculate which series pass the /// specified predicate. struct SeriesVisitor { @@ -843,10 +652,6 @@ mod tests { type TestError = Box; type Result = std::result::Result; - fn to_set(v: &[&str]) -> BTreeSet { - v.iter().map(|s| s.to_string()).collect::>() - } - #[tokio::test] async fn missing_tags_are_null() -> Result { let db = MutableBufferDb::new("mydb"); @@ -906,158 +711,6 @@ mod tests { Ok(()) } - #[tokio::test] - async fn list_column_values() -> Result { - let db = MutableBufferDb::new("column_namedb"); - - let lp_data = "h2o,state=CA,city=LA temp=70.4 100\n\ - h2o,state=MA,city=Boston temp=72.4 250\n\ - o2,state=MA,city=Boston temp=50.4 200\n\ - o2,state=CA temp=79.0 300\n\ - o2,state=NY temp=60.8 400\n"; - - let lines: Vec<_> = parse_lines(lp_data).map(|l| l.unwrap()).collect(); - write_lines(&db, &lines).await; - - #[derive(Debug)] - struct TestCase<'a> { - description: &'a str, - column_name: &'a str, - predicate: Predicate, - expected_column_values: Result>, - } - - let test_cases = vec![ - TestCase { - description: "No predicates, 'state' col", - column_name: "state", - predicate: PredicateBuilder::default().build(), - expected_column_values: Ok(vec!["CA", "MA", "NY"]), - }, - TestCase { - description: "No predicates, 'city' col", - column_name: "city", - predicate: PredicateBuilder::default().build(), - expected_column_values: Ok(vec!["Boston", "LA"]), - }, - TestCase { - description: "Restrictions: timestamp", - column_name: "state", - predicate: PredicateBuilder::default().timestamp_range(50, 201).build(), - expected_column_values: Ok(vec!["CA", "MA"]), - }, - TestCase { - description: "Restrictions: predicate", - column_name: "city", - predicate: PredicateBuilder::default() - .add_expr(col("state").eq(lit("MA"))) // state=MA - .build(), - expected_column_values: Ok(vec!["Boston"]), - }, - TestCase { - description: "Restrictions: timestamp and predicate", - column_name: "state", - predicate: PredicateBuilder::default() - .timestamp_range(150, 301) - .add_expr(col("state").eq(lit("MA"))) // state=MA - .build(), - expected_column_values: Ok(vec!["MA"]), - }, - TestCase { - description: "Restrictions: measurement name", - column_name: "state", - predicate: PredicateBuilder::default().table("h2o").build(), - expected_column_values: Ok(vec!["CA", "MA"]), - }, - TestCase { - description: "Restrictions: measurement name, with nulls", - column_name: "city", - predicate: PredicateBuilder::default().table("o2").build(), - expected_column_values: Ok(vec!["Boston"]), - }, - TestCase { - description: "Restrictions: measurement name and timestamp", - column_name: "state", - predicate: PredicateBuilder::default() - .table("o2") - .timestamp_range(50, 201) - .build(), - expected_column_values: Ok(vec!["MA"]), - }, - TestCase { - description: "Restrictions: measurement name and predicate", - column_name: "state", - predicate: PredicateBuilder::default() - .table("o2") - .add_expr(col("state").eq(lit("NY"))) // state=NY - .build(), - expected_column_values: Ok(vec!["NY"]), - }, - TestCase { - description: "Restrictions: measurement name, timestamp and predicate", - column_name: "state", - predicate: PredicateBuilder::default() - .table("o2") - .timestamp_range(1, 550) - .add_expr(col("state").eq(lit("NY"))) // state=NY - .build(), - expected_column_values: Ok(vec!["NY"]), - }, - TestCase { - description: "Restrictions: measurement name, timestamp and predicate: no match", - column_name: "state", - predicate: PredicateBuilder::default() - .table("o2") - .timestamp_range(1, 300) // filters out the NY row - .add_expr(col("state").eq(lit("NY"))) // state=NY - .build(), - expected_column_values: Ok(vec![]), - }, - ]; - - for test_case in test_cases.into_iter() { - let test_case_str = format!("{:#?}", test_case); - println!("Running test case: {:?}", test_case); - - let column_values_plan = db - .column_values(test_case.column_name, test_case.predicate) - .await - .expect("Created tag_values plan successfully"); - - // run the execution plan ( - let executor = Executor::default(); - let actual_column_values = executor.to_string_set(column_values_plan).await; - - let is_match = if let Ok(expected_column_values) = &test_case.expected_column_values { - let expected_column_values = to_set(expected_column_values); - if let Ok(actual_column_values) = &actual_column_values { - **actual_column_values == expected_column_values - } else { - false - } - } else if let Err(e) = &actual_column_values { - // use string compare to compare errors to avoid having to build exact errors - format!("{:?}", e) == format!("{:?}", test_case.expected_column_values) - } else { - false - }; - - assert!( - is_match, - "Mismatch\n\ - actual_column_values: \n\ - {:?}\n\ - expected_column_values: \n\ - {:?}\n\ - Test_case: \n\ - {}", - actual_column_values, test_case.expected_column_values, test_case_str - ); - } - - Ok(()) - } - #[tokio::test] async fn test_query_series() -> Result { // This test checks that everything is wired together @@ -1088,7 +741,7 @@ mod tests { let plans = db .query_series(predicate) .await - .expect("Created tag_values plan successfully"); + .expect("Created query_series plan successfully"); let results = run_and_gather_results(plans).await; @@ -1164,7 +817,7 @@ mod tests { let plans = db .query_series(predicate) .await - .expect("Created tag_values plan successfully"); + .expect("Created query_series plan successfully"); let results = run_and_gather_results(plans).await; @@ -1207,7 +860,7 @@ mod tests { let plans = db .query_series(predicate) .await - .expect("Created tag_values plan successfully"); + .expect("Created query_series plan successfully"); let results = run_and_gather_results(plans).await; assert!(results.is_empty()); @@ -1220,7 +873,7 @@ mod tests { let plans = db .query_series(predicate) .await - .expect("Created tag_values plan successfully"); + .expect("Created query_series plan successfully"); let results = run_and_gather_results(plans).await; assert_eq!(results.len(), 1); @@ -1234,7 +887,7 @@ mod tests { let plans = db .query_series(predicate) .await - .expect("Created tag_values plan successfully"); + .expect("Created query_series plan successfully"); let results = run_and_gather_results(plans).await; assert!(results.is_empty()); diff --git a/mutable_buffer/src/table.rs b/mutable_buffer/src/table.rs index 6a923f4ce8..ae54691ce9 100644 --- a/mutable_buffer/src/table.rs +++ b/mutable_buffer/src/table.rs @@ -35,7 +35,7 @@ use arrow_deps::{ }, datafusion::{ self, - logical_plan::{Expr, LogicalPlan, LogicalPlanBuilder}, + logical_plan::{Expr, LogicalPlanBuilder}, prelude::*, }, }; @@ -223,7 +223,7 @@ impl Table { } /// Returns a reference to the specified column - fn column(&self, column_id: u32) -> Result<&Column> { + pub(crate) fn column(&self, column_id: u32) -> Result<&Column> { self.columns.get(&column_id).context(ColumnIdNotFound { id: column_id, table_id: self.id, @@ -271,32 +271,6 @@ impl Table { } } - /// Creates a DataFusion LogicalPlan that returns column *values* as a - /// single column of Strings - /// - /// The created plan looks like: - /// - /// Projection - /// Filter(predicate) - /// InMemoryScan - pub fn tag_values_plan( - &self, - column_name: &str, - chunk_predicate: &ChunkPredicate, - chunk: &Chunk, - ) -> Result { - // Scan and Filter - let plan_builder = self.scan_with_predicates(chunk_predicate, chunk)?; - - let select_exprs = vec![col(column_name)]; - - plan_builder - .project(&select_exprs) - .context(BuildingPlan)? - .build() - .context(BuildingPlan) - } - /// Creates a SeriesSet plan that produces an output table with rows that /// match the predicate /// @@ -503,10 +477,7 @@ impl Table { column_name: col_name, chunk: chunk.id, })?; - let column = self.columns.get(&column_id).context(ColumnIdNotFound { - id: column_id, - table_id: self.id, - })?; + let column = self.column(column_id)?; Ok(column.data_type()) })?; @@ -735,10 +706,7 @@ impl Table { for col in &selection.cols { let column_name = col.column_name; - let column = self.columns.get(&col.column_id).context(ColumnIdNotFound { - id: col.column_id, - table_id: self.id, - })?; + let column = self.column(col.column_id)?; schema_builder = match column { Column::String(_, _) => schema_builder.field(column_name, ArrowDataType::Utf8), @@ -769,10 +737,7 @@ impl Table { let mut columns = Vec::with_capacity(selection.cols.len()); for col in &selection.cols { - let column = self.columns.get(&col.column_id).context(ColumnIdNotFound { - id: col.column_id, - table_id: self.id, - })?; + let column = self.column(col.column_id)?; let array = match column { Column::String(vals, _) => { @@ -1221,6 +1186,7 @@ impl<'a> TableColSelection<'a> { mod tests { use arrow::util::pretty::pretty_format_batches; + use arrow_deps::datafusion::logical_plan::LogicalPlan; use data_types::data::split_lines_into_write_entry_partitions; use influxdb_line_protocol::{parse_lines, ParsedLine}; use query::{ diff --git a/query/src/frontend/influxrpc.rs b/query/src/frontend/influxrpc.rs index 55121a1ba4..51014813d0 100644 --- a/query/src/frontend/influxrpc.rs +++ b/query/src/frontend/influxrpc.rs @@ -3,16 +3,21 @@ use std::{ sync::Arc, }; -use arrow_deps::datafusion::{ - error::{DataFusionError, Result as DatafusionResult}, - logical_plan::{Expr, ExpressionVisitor, LogicalPlan, LogicalPlanBuilder, Operator, Recursion}, - prelude::col, +use arrow_deps::{ + arrow::datatypes::DataType, + datafusion::{ + error::{DataFusionError, Result as DatafusionResult}, + logical_plan::{ + Expr, ExpressionVisitor, LogicalPlan, LogicalPlanBuilder, Operator, Recursion, + }, + prelude::col, + }, }; use data_types::{ schema::{InfluxColumnType, Schema}, selection::Selection, }; -use snafu::{OptionExt, ResultExt, Snafu}; +use snafu::{ensure, OptionExt, ResultExt, Snafu}; use tracing::debug; use crate::{ @@ -44,6 +49,11 @@ pub enum Error { source: Box, }, + #[snafu(display("gRPC planner got error finding column values: {}", source))] + FindingColumnValues { + source: Box, + }, + #[snafu(display( "gRPC planner got internal error making table_name with default predicate: {}", source @@ -68,7 +78,7 @@ pub enum Error { source: Box, }, - #[snafu(display("gRPC planner got error creating string set: {}", source))] + #[snafu(display("gRPC planner got error creating string set plan: {}", source))] CreatingStringSet { source: StringSetError }, #[snafu(display( @@ -81,13 +91,13 @@ pub enum Error { source: crate::provider::Error, }, - #[snafu(display("Error building plan: {}", source))] + #[snafu(display("gRPC planner got error building plan: {}", source))] BuildingPlan { source: arrow_deps::datafusion::error::DataFusionError, }, #[snafu(display( - "Error getting table schema for table '{}' in chunk {}: {}", + "gRPC planner got error getting table schema for table '{}' in chunk {}: {}", table_name, chunk_id, source @@ -98,8 +108,28 @@ pub enum Error { source: Box, }, - #[snafu(display("Unsupported predicate: {}", source))] + #[snafu(display("gRPC planner error: unsupported predicate: {}", source))] UnsupportedPredicate { source: DataFusionError }, + + #[snafu(display( + "gRPC planner error: column '{}' is not a tag, it is {:?}", + tag_name, + influx_column_type + ))] + InvalidTagColumn { + tag_name: String, + influx_column_type: Option, + }, + + #[snafu(display( + "Internal error: tag column '{}' is not Utf8 type, it is {:?} ", + tag_name, + data_type + ))] + InternalInvalidTagType { + tag_name: String, + data_type: DataType, + }, } pub type Result = std::result::Result; @@ -263,6 +293,155 @@ impl InfluxRPCPlanner { .context(CreatingStringSet) } + /// Returns a plan which finds the distinct, non-null tag values + /// in the specified `tag_name` column of this database which pass + /// the conditions specified by `predicate`. + pub async fn tag_values( + &self, + database: &D, + tag_name: &str, + predicate: Predicate, + ) -> Result + where + D: Database + 'static, + { + debug!(predicate=?predicate, tag_name, "planning tag_values"); + + // The basic algorithm is: + // + // 1. Find all the potential tables in the chunks + // + // 2. For each table/chunk pair, figure out which have + // distinct values that can be found from only metadata and + // which need full plans + + // Key is table name, value is set of chunks which had data + // for that table but that we couldn't evaluate the predicate + // entirely using the metadata + let mut need_full_plans = BTreeMap::new(); + + let mut known_values = BTreeSet::new(); + for chunk in self.filtered_chunks(database, &predicate).await? { + let table_names = self.chunk_table_names(chunk.as_ref(), &predicate).await?; + + for table_name in table_names { + debug!( + table_name = table_name.as_str(), + chunk_id = chunk.id(), + "finding columns in table" + ); + + // use schema to validate column type + let schema = chunk + .table_schema(&table_name, Selection::All) + .await + .expect("to be able to get table schema"); + + // Skip this table if the tag_name is not a column in this table + let idx = if let Some(idx) = schema.find_index_of(tag_name) { + idx + } else { + continue; + }; + + // Validate that this really is a Tag column + let (influx_column_type, field) = schema.field(idx); + ensure!( + matches!(influx_column_type, Some(InfluxColumnType::Tag)), + InvalidTagColumn { + tag_name, + influx_column_type, + } + ); + ensure!( + field.data_type() == &DataType::Utf8, + InternalInvalidTagType { + tag_name, + data_type: field.data_type().clone(), + } + ); + + // try and get the list of values directly from metadata + let maybe_values = chunk + .column_values(&table_name, tag_name, &predicate) + .await + .map_err(|e| Box::new(e) as _) + .context(FindingColumnValues)?; + + match maybe_values { + Some(mut names) => { + debug!(names=?names, chunk_id = chunk.id(), "column values found from metadata"); + known_values.append(&mut names); + } + None => { + debug!( + table_name = table_name.as_str(), + chunk_id = chunk.id(), + "need full plan to find column values" + ); + // can't get columns only from metadata, need + // a general purpose plan + need_full_plans + .entry(table_name) + .or_insert_with(Vec::new) + .push(Arc::clone(&chunk)); + } + } + } + } + + let mut builder = StringSetPlanBuilder::new(); + + let select_exprs = vec![col(tag_name)]; + + // At this point, we have a set of tag_values we know at plan + // time in `known_columns`, and some tables in chunks that we + // need to run a plan to find what values pass the predicate. + for (table_name, chunks) in need_full_plans.into_iter() { + let scan_and_filter = self + .scan_and_filter(&table_name, &predicate, chunks) + .await?; + + // if we have any data to scan, make a plan! + if let Some(TableScanAndFilter { + plan_builder, + schema: _, + }) = scan_and_filter + { + // TODO use Expr::is_null() here when this + // https://issues.apache.org/jira/browse/ARROW-11742 + // is completed. + let tag_name_is_not_null = Expr::IsNotNull(Box::new(col(tag_name))); + + // TODO: optimize this to use "DISINCT" or do + // something more intelligent that simply fetching all + // the values and reducing them in the query Executor + // + // Until then, simply use a plan which looks like: + // + // Projection + // Filter(is not null) + // Filter(predicate) + // InMemoryScan + let plan = plan_builder + .project(&select_exprs) + .context(BuildingPlan)? + .filter(tag_name_is_not_null) + .context(BuildingPlan)? + .build() + .context(BuildingPlan)?; + + builder = builder.append(plan.into()); + } + } + + // add the known values we could find from metadata only + builder + .append(known_values.into()) + .build() + .context(CreatingStringSet) + } + /// Returns a plan that produces a list of columns and their /// datatypes (as defined in the data written via `write_lines`), /// and which have more than zero rows which pass the conditions diff --git a/query/src/lib.rs b/query/src/lib.rs index 95308bc8ca..9ab66c9155 100644 --- a/query/src/lib.rs +++ b/query/src/lib.rs @@ -12,7 +12,6 @@ use data_types::{ data::ReplicatedWrite, partition_metadata::TableSummary, schema::Schema, selection::Selection, }; use exec::{stringset::StringSet, Executor, SeriesSetPlans}; -use plan::stringset::StringSetPlan; use std::{fmt::Debug, sync::Arc}; @@ -55,15 +54,6 @@ pub trait Database: Debug + Send + Sync { // The functions below are slated for removal (migration into a gRPC query // frontend) --------- - /// Returns a plan which finds the distinct values in the - /// `column_name` column of this database which pass the - /// conditions specified by `predicate`. - async fn column_values( - &self, - column_name: &str, - predicate: Predicate, - ) -> Result; - /// Returns a plan that finds all rows rows which pass the /// conditions specified by `predicate` in the form of logical /// time series. @@ -132,13 +122,25 @@ pub trait PartitionChunk: Debug + Send + Sync { /// Returns a set of Strings with column names from the specified /// table that have at least one row that matches `predicate`, if /// the predicate can be evaluated entirely on the metadata of - /// this Chunk. + /// this Chunk. Returns `None` otherwise async fn column_names( &self, table_name: &str, predicate: &Predicate, ) -> Result, Self::Error>; + /// Return a set of Strings containing the distinct values in the + /// specified columns. If the predicate can be evaluated entirely + /// on the metadata of this Chunk. Returns `None` otherwise + /// + /// The requested columns must all have String type. + async fn column_values( + &self, + table_name: &str, + column_name: &str, + predicate: &Predicate, + ) -> Result, Self::Error>; + /// Returns the Schema for a table in this chunk, with the /// specified column selection. An error is returned if the /// selection refers to columns that do not exist. diff --git a/query/src/test.rs b/query/src/test.rs index 348ca4f8a3..0329c5c53f 100644 --- a/query/src/test.rs +++ b/query/src/test.rs @@ -12,7 +12,7 @@ use arrow_deps::{ datafusion::physical_plan::{common::SizedRecordBatchStream, SendableRecordBatchStream}, }; -use crate::{exec::Executor, group_by::GroupByAndAggregate, plan::stringset::StringSetPlan}; +use crate::{exec::Executor, group_by::GroupByAndAggregate}; use crate::{ exec::{ stringset::{StringSet, StringSetRef}, @@ -59,12 +59,6 @@ pub struct TestDatabase { /// `column_names` to return upon next request column_names: Arc>>, - /// `column_values` to return upon next request - column_values: Arc>>, - - /// The last request for `column_values` - column_values_request: Arc>>, - /// Responses to return on the next request to `query_series` query_series_values: Arc>>, @@ -78,16 +72,6 @@ pub struct TestDatabase { query_groups_request: Arc>>, } -/// Records the parameters passed to a column values request -#[derive(Debug, PartialEq, Clone)] -pub struct ColumnValuesRequest { - /// The name of the requested column - pub column_name: String, - - /// Stringified '{:?}' version of the predicate - pub predicate: String, -} - /// Records the parameters passed to a `query_series` request #[derive(Debug, PartialEq, Clone)] pub struct QuerySeriesRequest { @@ -178,20 +162,6 @@ impl TestDatabase { *Arc::clone(&self.column_names).lock() = Some(column_names) } - /// Set the list of column values that will be returned on a call to - /// column_values - pub fn set_column_values(&self, column_values: Vec) { - let column_values = column_values.into_iter().collect::(); - let column_values = Arc::new(column_values); - - *Arc::clone(&self.column_values).lock() = Some(column_values) - } - - /// Get the parameters from the last column name request - pub fn get_column_values_request(&self) -> Option { - Arc::clone(&self.column_values_request).lock().take() - } - /// Set the series that will be returned on a call to query_series pub fn set_query_series_values(&self, plan: SeriesSetPlans) { *Arc::clone(&self.query_series_values).lock() = Some(plan); @@ -267,34 +237,6 @@ impl Database for TestDatabase { Ok(()) } - /// Return the mocked out column values, recording the request - async fn column_values( - &self, - column_name: &str, - predicate: Predicate, - ) -> Result { - // save the request - let predicate = predicate_to_test_string(&predicate); - - let new_column_values_request = Some(ColumnValuesRequest { - column_name: column_name.into(), - predicate, - }); - - *Arc::clone(&self.column_values_request).lock() = new_column_values_request; - - // pull out the saved columns - let column_values = Arc::clone(&self.column_values) - .lock() - .take() - // Turn None into an error - .context(General { - message: "No saved column_values in TestDatabase", - })?; - - Ok(StringSetPlan::Known(column_values)) - } - async fn query_series(&self, predicate: Predicate) -> Result { let predicate = predicate_to_test_string(&predicate); @@ -566,6 +508,16 @@ impl PartitionChunk for TestChunk { }) } + async fn column_values( + &self, + _table_name: &str, + _column_name: &str, + _predicate: &Predicate, + ) -> Result, Self::Error> { + // Model not being able to get column values from metadata + Ok(None) + } + fn has_table(&self, table_name: &str) -> bool { self.table_schemas.contains_key(table_name) } diff --git a/server/src/db.rs b/server/src/db.rs index 4c2c0f105d..c8ca7cf150 100644 --- a/server/src/db.rs +++ b/server/src/db.rs @@ -13,7 +13,7 @@ use async_trait::async_trait; use data_types::{data::ReplicatedWrite, database_rules::DatabaseRules, selection::Selection}; use mutable_buffer::MutableBufferDb; use parking_lot::Mutex; -use query::{plan::stringset::StringSetPlan, Database, PartitionChunk}; +use query::{Database, PartitionChunk}; use read_buffer::Database as ReadBufferDb; use serde::{Deserialize, Serialize}; use snafu::{OptionExt, ResultExt, Snafu}; @@ -306,19 +306,6 @@ impl Database for Db { .context(MutableBufferWrite) } - async fn column_values( - &self, - column_name: &str, - predicate: query::predicate::Predicate, - ) -> Result { - self.mutable_buffer - .as_ref() - .context(DatabaseNotReadable)? - .column_values(column_name, predicate) - .await - .context(MutableBufferRead) - } - async fn query_series( &self, predicate: query::predicate::Predicate, diff --git a/server/src/db/chunk.rs b/server/src/db/chunk.rs index 1d60170ad5..7c4e237d8e 100644 --- a/server/src/db/chunk.rs +++ b/server/src/db/chunk.rs @@ -355,4 +355,40 @@ impl PartitionChunk for DBChunk { } } } + + async fn column_values( + &self, + table_name: &str, + column_name: &str, + predicate: &Predicate, + ) -> Result, Self::Error> { + match self { + Self::MutableBuffer { chunk } => { + use mutable_buffer::chunk::Error::UnsupportedColumnTypeForListingValues; + + let chunk_predicate = chunk + .compile_predicate(predicate) + .context(MutableBufferChunk)?; + + let values = chunk.tag_column_values(table_name, column_name, &chunk_predicate); + + // if the mutable buffer doesn't support getting + // values for this kind of column, report back None + if let Err(UnsupportedColumnTypeForListingValues { .. }) = values { + Ok(None) + } else { + values.context(MutableBufferChunk) + } + } + Self::ReadBuffer { .. } => { + // TODO hook up read buffer API here when ready. Until + // now, fallback to using a full plan + // https://github.com/influxdata/influxdb_iox/issues/857 + Ok(None) + } + Self::ParquetFile => { + unimplemented!("parquet file not implemented for column_values") + } + } + } } diff --git a/server/src/query_tests/influxrpc.rs b/server/src/query_tests/influxrpc.rs index 8990b75f58..665cf098aa 100644 --- a/server/src/query_tests/influxrpc.rs +++ b/server/src/query_tests/influxrpc.rs @@ -1,3 +1,4 @@ pub mod field_columns; pub mod table_names; pub mod tag_keys; +pub mod tag_values; diff --git a/server/src/query_tests/influxrpc/tag_values.rs b/server/src/query_tests/influxrpc/tag_values.rs new file mode 100644 index 0000000000..484dc41ab3 --- /dev/null +++ b/server/src/query_tests/influxrpc/tag_values.rs @@ -0,0 +1,253 @@ +use arrow_deps::datafusion::logical_plan::{col, lit}; +use query::{ + exec::{ + stringset::{IntoStringSet, StringSetRef}, + Executor, + }, + frontend::influxrpc::InfluxRPCPlanner, + predicate::PredicateBuilder, +}; + +use crate::query_tests::scenarios::*; + +/// runs tag_value(predicate) and compares it to the expected +/// output +macro_rules! run_tag_values_test_case { + ($DB_SETUP:expr, $TAG_NAME:expr, $PREDICATE:expr, $EXPECTED_VALUES:expr) => { + test_helpers::maybe_start_logging(); + let predicate = $PREDICATE; + let tag_name = $TAG_NAME; + let expected_values = $EXPECTED_VALUES; + for scenario in $DB_SETUP.make().await { + let DBScenario { + scenario_name, db, .. + } = scenario; + println!("Running scenario '{}'", scenario_name); + println!("Predicate: '{:#?}'", predicate); + let planner = InfluxRPCPlanner::new(); + let executor = Executor::new(); + + let plan = planner + .tag_values(&db, &tag_name, predicate.clone()) + .await + .expect("built plan successfully"); + let names = executor + .to_string_set(plan) + .await + .expect("converted plan to strings successfully"); + + assert_eq!( + names, + to_stringset(&expected_values), + "Error in scenario '{}'\n\nexpected:\n{:?}\nactual:\n{:?}", + scenario_name, + expected_values, + names + ); + } + }; +} + +#[tokio::test] +async fn list_tag_values_no_tag() { + let predicate = PredicateBuilder::default().build(); + // If the tag is not present, expect no values back (not error) + let tag_name = "tag_not_in_chunks"; + let expected_tag_keys = vec![]; + run_tag_values_test_case!( + TwoMeasurementsManyNulls {}, + tag_name, + predicate, + expected_tag_keys + ); +} + +#[tokio::test] +async fn list_tag_values_no_predicate_state_col() { + let predicate = PredicateBuilder::default().build(); + let tag_name = "state"; + let expected_tag_keys = vec!["CA", "MA", "NY"]; + run_tag_values_test_case!( + TwoMeasurementsManyNulls {}, + tag_name, + predicate, + expected_tag_keys + ); +} + +#[tokio::test] +async fn list_tag_values_no_predicate_city_col() { + let tag_name = "city"; + let predicate = PredicateBuilder::default().build(); + let expected_tag_keys = vec!["Boston", "LA", "NYC"]; + run_tag_values_test_case!( + TwoMeasurementsManyNulls {}, + tag_name, + predicate, + expected_tag_keys + ); +} + +#[tokio::test] +async fn list_tag_values_timestamp_pred_state_col() { + let tag_name = "state"; + let predicate = PredicateBuilder::default().timestamp_range(50, 201).build(); + let expected_tag_keys = vec!["CA", "MA"]; + run_tag_values_test_case!( + TwoMeasurementsManyNulls {}, + tag_name, + predicate, + expected_tag_keys + ); +} + +#[tokio::test] +async fn list_tag_values_state_pred_state_col() { + let tag_name = "city"; + let predicate = PredicateBuilder::default() + .add_expr(col("state").eq(lit("MA"))) // state=MA + .build(); + let expected_tag_keys = vec!["Boston"]; + run_tag_values_test_case!( + TwoMeasurementsManyNulls {}, + tag_name, + predicate, + expected_tag_keys + ); +} + +#[tokio::test] +async fn list_tag_values_timestamp_and_state_pred_state_col() { + let tag_name = "state"; + let predicate = PredicateBuilder::default() + .timestamp_range(150, 301) + .add_expr(col("state").eq(lit("MA"))) // state=MA + .build(); + let expected_tag_keys = vec!["MA"]; + run_tag_values_test_case!( + TwoMeasurementsManyNulls {}, + tag_name, + predicate, + expected_tag_keys + ); +} + +#[tokio::test] +async fn list_tag_values_table_pred_state_col() { + let tag_name = "state"; + let predicate = PredicateBuilder::default().table("h2o").build(); + let expected_tag_keys = vec!["CA", "MA"]; + run_tag_values_test_case!( + TwoMeasurementsManyNulls {}, + tag_name, + predicate, + expected_tag_keys + ); +} + +#[tokio::test] +async fn list_tag_values_table_pred_city_col() { + let tag_name = "city"; + let predicate = PredicateBuilder::default().table("o2").build(); + let expected_tag_keys = vec!["Boston", "NYC"]; + run_tag_values_test_case!( + TwoMeasurementsManyNulls {}, + tag_name, + predicate, + expected_tag_keys + ); +} + +#[tokio::test] +async fn list_tag_values_table_and_timestamp_and_table_pred_state_col() { + let tag_name = "state"; + let predicate = PredicateBuilder::default() + .table("o2") + .timestamp_range(50, 201) + .build(); + let expected_tag_keys = vec!["MA"]; + run_tag_values_test_case!( + TwoMeasurementsManyNulls {}, + tag_name, + predicate, + expected_tag_keys + ); +} + +#[tokio::test] +async fn list_tag_values_table_and_state_pred_state_col() { + let tag_name = "state"; + let predicate = PredicateBuilder::default() + .table("o2") + .add_expr(col("state").eq(lit("NY"))) // state=NY + .build(); + let expected_tag_keys = vec!["NY"]; + run_tag_values_test_case!( + TwoMeasurementsManyNulls {}, + tag_name, + predicate, + expected_tag_keys + ); +} + +#[tokio::test] +async fn list_tag_values_table_and_timestamp_and_state_pred_state_col() { + let tag_name = "state"; + let predicate = PredicateBuilder::default() + .table("o2") + .timestamp_range(1, 550) + .add_expr(col("state").eq(lit("NY"))) // state=NY + .build(); + let expected_tag_keys = vec!["NY"]; + run_tag_values_test_case!( + TwoMeasurementsManyNulls {}, + tag_name, + predicate, + expected_tag_keys + ); +} + +#[tokio::test] +async fn list_tag_values_table_and_timestamp_and_state_pred_state_col_no_rows() { + let tag_name = "state"; + let predicate = PredicateBuilder::default() + .table("o2") + .timestamp_range(1, 300) // filters out the NY row + .add_expr(col("state").eq(lit("NY"))) // state=NY + .build(); + let expected_tag_keys = vec![]; + run_tag_values_test_case!( + TwoMeasurementsManyNulls {}, + tag_name, + predicate, + expected_tag_keys + ); +} + +#[tokio::test] +async fn list_tag_values_field_col() { + let db_setup = TwoMeasurementsManyNulls {}; + let predicate = PredicateBuilder::default().build(); + + for scenario in db_setup.make().await { + let DBScenario { + scenario_name, db, .. + } = scenario; + println!("Running scenario '{}'", scenario_name); + println!("Predicate: '{:#?}'", predicate); + let planner = InfluxRPCPlanner::new(); + + // Test: temp is a field, not a tag + let tag_name = "temp"; + let plan_result = planner.tag_values(&db, &tag_name, predicate.clone()).await; + + assert_eq!( + plan_result.unwrap_err().to_string(), + "gRPC planner error: column \'temp\' is not a tag, it is Some(Field(Float))" + ); + } +} + +fn to_stringset(v: &[&str]) -> StringSetRef { + v.into_stringset().unwrap() +} diff --git a/src/influxdb_ioxd/rpc/storage/service.rs b/src/influxdb_ioxd/rpc/storage/service.rs index 1fa9323170..f36f85fa38 100644 --- a/src/influxdb_ioxd/rpc/storage/service.rs +++ b/src/influxdb_ioxd/rpc/storage/service.rs @@ -852,7 +852,7 @@ async fn tag_values_impl( rpc_predicate: Option, ) -> Result where - T: DatabaseStore, + T: DatabaseStore + 'static, { let rpc_predicate_string = format!("{:?}", rpc_predicate); @@ -873,10 +873,12 @@ where .await .context(DatabaseNotFound { db_name })?; + let planner = InfluxRPCPlanner::new(); + let executor = db_store.executor(); - let tag_value_plan = db - .column_values(tag_name, predicate) + let tag_value_plan = planner + .tag_values(db.as_ref(), tag_name, predicate) .await .map_err(|e| Box::new(e) as _) .context(ListingTagValues { db_name, tag_name })?; @@ -1111,7 +1113,7 @@ mod tests { group_by::{Aggregate as QueryAggregate, WindowDuration as QueryWindowDuration}, test::QueryGroupsRequest, test::TestDatabaseStore, - test::{ColumnValuesRequest, QuerySeriesRequest, TestChunk}, + test::{QuerySeriesRequest, TestChunk}, }; use std::{ convert::TryFrom, @@ -1478,11 +1480,18 @@ mod tests { let db_info = OrgAndBucket::new(123, 456); let partition_id = 1; - let test_db = fixture + // Add a chunk with a field + let chunk = TestChunk::new(0) + .with_time_column("TheMeasurement") + .with_tag_column("TheMeasurement", "state") + .with_one_row_of_null_data("TheMeasurement"); + + fixture .test_storage .db_or_create(&db_info.db_name) .await - .expect("creating test database"); + .unwrap() + .add_chunk("my_partition_key", Arc::new(chunk)); let source = Some(StorageClientWrapper::read_source( db_info.org_id, @@ -1490,24 +1499,35 @@ mod tests { partition_id, )); - let tag_values = vec!["k1", "k2", "k3", "k4"]; let request = TagValuesRequest { tags_source: source.clone(), - range: make_timestamp_range(150, 200), + range: make_timestamp_range(150, 2000), predicate: make_state_ma_predicate(), - tag_key: "the_tag_key".into(), + tag_key: "state".into(), }; - let expected_request = ColumnValuesRequest { - predicate: "Predicate { exprs: [#state Eq Utf8(\"MA\")] range: TimestampRange { start: 150, end: 200 }}".into(), - column_name: "the_tag_key".into(), - }; - - test_db.set_column_values(to_string_vec(&tag_values)); - let actual_tag_values = fixture.storage_client.tag_values(request).await.unwrap(); - assert_eq!(actual_tag_values, tag_values,); - assert_eq!(test_db.get_column_values_request(), Some(expected_request),); + assert_eq!(actual_tag_values, vec!["MA"]); + } + + /// test the plumbing of the RPC layer for tag_values + /// + /// For the special case of + /// + /// tag_key = _measurement means listing all measurement names + #[tokio::test] + async fn test_storage_rpc_tag_values_with_measurement() { + // Start a test gRPC server on a randomally allocated port + let mut fixture = Fixture::new().await.expect("Connecting to test server"); + + let db_info = OrgAndBucket::new(123, 456); + let partition_id = 1; + + let source = Some(StorageClientWrapper::read_source( + db_info.org_id, + db_info.bucket_id, + partition_id, + )); // --- // test tag_key = _measurement means listing all measurement names @@ -1590,11 +1610,14 @@ mod tests { let db_info = OrgAndBucket::new(123, 456); let partition_id = 1; - let test_db = fixture + let chunk = TestChunk::new(0).with_error("Sugar we are going down"); + + fixture .test_storage .db_or_create(&db_info.db_name) .await - .expect("creating test database"); + .unwrap() + .add_chunk("my_partition_key", Arc::new(chunk)); let source = Some(StorageClientWrapper::read_source( db_info.org_id, @@ -1612,12 +1635,13 @@ mod tests { tag_key: "the_tag_key".into(), }; - // Note we don't set the column_names on the test database, so we expect an - // error - let response = fixture.storage_client.tag_values(request).await; - assert!(response.is_err()); - let response_string = format!("{:?}", response); - let expected_error = "No saved column_values in TestDatabase"; + let response_string = fixture + .storage_client + .tag_values(request) + .await + .unwrap_err() + .to_string(); + let expected_error = "Sugar we are going down"; assert!( response_string.contains(expected_error), "'{}' did not contain expected content '{}'", @@ -1625,12 +1649,6 @@ mod tests { expected_error ); - let expected_request = Some(ColumnValuesRequest { - predicate: "Predicate {}".into(), - column_name: "the_tag_key".into(), - }); - assert_eq!(test_db.get_column_values_request(), expected_request); - // --- // test error with non utf8 value // --- @@ -1641,9 +1659,12 @@ mod tests { tag_key: [0, 255].into(), // this is not a valid UTF-8 string }; - let response = fixture.storage_client.tag_values(request).await; - assert!(response.is_err()); - let response_string = format!("{:?}", response); + let response_string = fixture + .storage_client + .tag_values(request) + .await + .unwrap_err() + .to_string(); let expected_error = "Error converting tag_key to UTF-8 in tag_values request"; assert!( response_string.contains(expected_error), @@ -1653,22 +1674,27 @@ mod tests { ); } - /// test the plumbing of the RPC layer for measurement_tag_values-- - /// specifically that the right parameters are passed into the Database - /// interface and that the returned values are sent back via gRPC. + /// test the plumbing of the RPC layer for measurement_tag_values #[tokio::test] async fn test_storage_rpc_measurement_tag_values() { - // Start a test gRPC server on a randomally allocated port + test_helpers::maybe_start_logging(); let mut fixture = Fixture::new().await.expect("Connecting to test server"); let db_info = OrgAndBucket::new(123, 456); let partition_id = 1; - let test_db = fixture + // Add a chunk with a field + let chunk = TestChunk::new(0) + .with_time_column("TheMeasurement") + .with_tag_column("TheMeasurement", "state") + .with_one_row_of_null_data("TheMeasurement"); + + fixture .test_storage .db_or_create(&db_info.db_name) .await - .expect("creating test database"); + .unwrap() + .add_chunk("my_partition_key", Arc::new(chunk)); let source = Some(StorageClientWrapper::read_source( db_info.org_id, @@ -1676,22 +1702,14 @@ mod tests { partition_id, )); - let tag_values = vec!["k1", "k2", "k3", "k4"]; let request = MeasurementTagValuesRequest { - measurement: "m4".into(), + measurement: "TheMeasurement".into(), source: source.clone(), - range: make_timestamp_range(150, 200), + range: make_timestamp_range(150, 2000), predicate: make_state_ma_predicate(), - tag_key: "the_tag_key".into(), + tag_key: "state".into(), }; - let expected_request = ColumnValuesRequest { - predicate: "Predicate { table_names: m4 exprs: [#state Eq Utf8(\"MA\")] range: TimestampRange { start: 150, end: 200 }}".into(), - column_name: "the_tag_key".into(), - }; - - test_db.set_column_values(to_string_vec(&tag_values)); - let actual_tag_values = fixture .storage_client .measurement_tag_values(request) @@ -1699,15 +1717,34 @@ mod tests { .unwrap(); assert_eq!( - actual_tag_values, tag_values, + actual_tag_values, + vec!["MA"], "unexpected tag values while getting tag values", ); + } - assert_eq!( - test_db.get_column_values_request(), - Some(expected_request), - "unexpected request while getting tag values", - ); + #[tokio::test] + async fn test_storage_rpc_measurement_tag_values_error() { + test_helpers::maybe_start_logging(); + let mut fixture = Fixture::new().await.expect("Connecting to test server"); + + let db_info = OrgAndBucket::new(123, 456); + let partition_id = 1; + + let chunk = TestChunk::new(0).with_error("Sugar we are going down"); + + fixture + .test_storage + .db_or_create(&db_info.db_name) + .await + .unwrap() + .add_chunk("my_partition_key", Arc::new(chunk)); + + let source = Some(StorageClientWrapper::read_source( + db_info.org_id, + db_info.bucket_id, + partition_id, + )); // --- // test error @@ -1722,22 +1759,19 @@ mod tests { // Note we don't set the column_names on the test database, so we expect an // error - let response = fixture.storage_client.measurement_tag_values(request).await; - assert!(response.is_err()); - let response_string = format!("{:?}", response); - let expected_error = "No saved column_values in TestDatabase"; + let response_string = fixture + .storage_client + .measurement_tag_values(request) + .await + .unwrap_err() + .to_string(); + let expected_error = "Sugar we are going down"; assert!( response_string.contains(expected_error), "'{}' did not contain expected content '{}'", response_string, expected_error ); - - let expected_request = Some(ColumnValuesRequest { - predicate: "Predicate { table_names: m5}".into(), - column_name: "the_tag_key".into(), - }); - assert_eq!(test_db.get_column_values_request(), expected_request); } #[tokio::test] From d29d7efa8cc004863670be10ad7d3a83552841b1 Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Thu, 25 Feb 2021 07:39:01 -0500 Subject: [PATCH 11/22] chore: Update arrow/datafusion deps again --- Cargo.lock | 196 +++++++++++++++++++++++++----------------- arrow_deps/Cargo.toml | 10 +-- 2 files changed, 120 insertions(+), 86 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6345cf0326..314ac0f885 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -101,7 +101,7 @@ checksum = "23b62fc65de8e4e7f52534fb52b0f3ed04746ae267519eef2a83941e8085068b" [[package]] name = "arrow" version = "4.0.0-SNAPSHOT" -source = "git+https://github.com/apache/arrow.git?rev=ad4504e8e85eb8e5babe0f01ca8cf9947499fc40#ad4504e8e85eb8e5babe0f01ca8cf9947499fc40" +source = "git+https://github.com/apache/arrow.git?rev=b5ac048c75cc55f4039d279f554920be3112d7cd#b5ac048c75cc55f4039d279f554920be3112d7cd" dependencies = [ "cfg_aliases", "chrono", @@ -124,7 +124,7 @@ dependencies = [ [[package]] name = "arrow-flight" version = "4.0.0-SNAPSHOT" -source = "git+https://github.com/apache/arrow.git?rev=ad4504e8e85eb8e5babe0f01ca8cf9947499fc40#ad4504e8e85eb8e5babe0f01ca8cf9947499fc40" +source = "git+https://github.com/apache/arrow.git?rev=b5ac048c75cc55f4039d279f554920be3112d7cd#b5ac048c75cc55f4039d279f554920be3112d7cd" dependencies = [ "arrow", "bytes", @@ -411,9 +411,9 @@ dependencies = [ [[package]] name = "bumpalo" -version = "3.6.0" +version = "3.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "099e596ef14349721d9016f6b80dd3419ea1bf289ab9b44df8e4dfd3a005d5d9" +checksum = "63396b8a4b9de3f4fdfb320ab6080762242f66a8ef174c49d8e19b674db4cdbe" [[package]] name = "byteorder" @@ -438,9 +438,9 @@ dependencies = [ [[package]] name = "cc" -version = "1.0.66" +version = "1.0.67" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4c0496836a84f8d0495758516b8621a622beb77c0fed418570e50764093ced48" +checksum = "e3c69b077ad434294d3ce9f1f6143a2a4b89a8a2d54ef813d85003a4fd1137fd" dependencies = [ "jobserver", ] @@ -488,9 +488,9 @@ dependencies = [ [[package]] name = "clang-sys" -version = "1.1.0" +version = "1.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5cb92721cb37482245ed88428f72253ce422b3b4ee169c70a0642521bb5db4cc" +checksum = "f54d78e30b388d4815220c8dd03fea5656b6c6d32adb59e89061552a102f8da1" dependencies = [ "glob", "libc", @@ -669,9 +669,9 @@ dependencies = [ "cfg-if 1.0.0", "crossbeam-channel 0.5.0", "crossbeam-deque 0.8.0", - "crossbeam-epoch 0.9.1", + "crossbeam-epoch 0.9.2", "crossbeam-queue 0.3.1", - "crossbeam-utils 0.8.1", + "crossbeam-utils 0.8.2", ] [[package]] @@ -691,7 +691,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dca26ee1f8d361640700bde38b2c37d8c22b3ce2d360e1fc1c74ea4b0aa7d775" dependencies = [ "cfg-if 1.0.0", - "crossbeam-utils 0.8.1", + "crossbeam-utils 0.8.2", ] [[package]] @@ -712,8 +712,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "94af6efb46fef72616855b036a624cf27ba656ffc9be1b9a3c931cfc7749a9a9" dependencies = [ "cfg-if 1.0.0", - "crossbeam-epoch 0.9.1", - "crossbeam-utils 0.8.1", + "crossbeam-epoch 0.9.2", + "crossbeam-utils 0.8.2", ] [[package]] @@ -733,14 +733,14 @@ dependencies = [ [[package]] name = "crossbeam-epoch" -version = "0.9.1" +version = "0.9.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a1aaa739f95311c2c7887a76863f500026092fb1dce0161dab577e559ef3569d" +checksum = "d60ab4a8dba064f2fbb5aa270c28da5cf4bbd0e72dae1140a6b0353a779dbe00" dependencies = [ "cfg-if 1.0.0", - "const_fn", - "crossbeam-utils 0.8.1", + "crossbeam-utils 0.8.2", "lazy_static", + "loom", "memoffset 0.6.1", "scopeguard", ] @@ -763,7 +763,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0f6cb3c7f5b8e51bc3ebb73a2327ad4abdbd119dc13223f14f961d2f38486756" dependencies = [ "cfg-if 1.0.0", - "crossbeam-utils 0.8.1", + "crossbeam-utils 0.8.2", ] [[package]] @@ -779,13 +779,14 @@ dependencies = [ [[package]] name = "crossbeam-utils" -version = "0.8.1" +version = "0.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "02d96d1e189ef58269ebe5b97953da3274d83a93af647c2ddd6f9dab28cedb8d" +checksum = "bae8f328835f8f5a6ceb6a7842a7f2d0c03692adb5c889347235d59194731fe3" dependencies = [ "autocfg", "cfg-if 1.0.0", "lazy_static", + "loom", ] [[package]] @@ -850,7 +851,7 @@ dependencies = [ [[package]] name = "datafusion" version = "4.0.0-SNAPSHOT" -source = "git+https://github.com/apache/arrow.git?rev=ad4504e8e85eb8e5babe0f01ca8cf9947499fc40#ad4504e8e85eb8e5babe0f01ca8cf9947499fc40" +source = "git+https://github.com/apache/arrow.git?rev=b5ac048c75cc55f4039d279f554920be3112d7cd#b5ac048c75cc55f4039d279f554920be3112d7cd" dependencies = [ "ahash 0.7.0", "arrow", @@ -871,6 +872,7 @@ dependencies = [ "sha2", "sqlparser 0.8.0", "tokio", + "unicode-segmentation", ] [[package]] @@ -1115,9 +1117,9 @@ checksum = "00b0228411908ca8685dba7fc2cdd70ec9990a6e753e89b6ac91a84c40fbaf4b" [[package]] name = "form_urlencoded" -version = "1.0.0" +version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ece68d15c92e84fa4f19d3780f1294e5ca82a78a6d515f1efaabcc144688be00" +checksum = "5fc25a87fa4fd2094bffb06925852034d90a17f0d1e05197d4956d3555752191" dependencies = [ "matches", "percent-encoding", @@ -1135,9 +1137,9 @@ dependencies = [ [[package]] name = "futures" -version = "0.3.12" +version = "0.3.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "da9052a1a50244d8d5aa9bf55cbc2fb6f357c86cc52e46c62ed390a7180cf150" +checksum = "7f55667319111d593ba876406af7c409c0ebb44dc4be6132a783ccf163ea14c1" dependencies = [ "futures-channel", "futures-core", @@ -1150,9 +1152,9 @@ dependencies = [ [[package]] name = "futures-channel" -version = "0.3.12" +version = "0.3.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f2d31b7ec7efab6eefc7c57233bb10b847986139d88cc2f5a02a1ae6871a1846" +checksum = "8c2dd2df839b57db9ab69c2c9d8f3e8c81984781937fe2807dc6dcf3b2ad2939" dependencies = [ "futures-core", "futures-sink", @@ -1160,15 +1162,15 @@ dependencies = [ [[package]] name = "futures-core" -version = "0.3.12" +version = "0.3.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "79e5145dde8da7d1b3892dad07a9c98fc04bc39892b1ecc9692cf53e2b780a65" +checksum = "15496a72fabf0e62bdc3df11a59a3787429221dd0710ba8ef163d6f7a9112c94" [[package]] name = "futures-executor" -version = "0.3.12" +version = "0.3.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e9e59fdc009a4b3096bf94f740a0f2424c082521f20a9b08c5c07c48d90fd9b9" +checksum = "891a4b7b96d84d5940084b2a37632dd65deeae662c114ceaa2c879629c9c0ad1" dependencies = [ "futures-core", "futures-task", @@ -1177,15 +1179,15 @@ dependencies = [ [[package]] name = "futures-io" -version = "0.3.12" +version = "0.3.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "28be053525281ad8259d47e4de5de657b25e7bac113458555bb4b70bc6870500" +checksum = "d71c2c65c57704c32f5241c1223167c2c3294fd34ac020c807ddbe6db287ba59" [[package]] name = "futures-macro" -version = "0.3.12" +version = "0.3.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c287d25add322d9f9abdcdc5927ca398917996600182178774032e9f8258fedd" +checksum = "ea405816a5139fb39af82c2beb921d52143f556038378d6db21183a5c37fbfb7" dependencies = [ "proc-macro-hack", "proc-macro2", @@ -1195,24 +1197,21 @@ dependencies = [ [[package]] name = "futures-sink" -version = "0.3.12" +version = "0.3.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "caf5c69029bda2e743fddd0582d1083951d65cc9539aebf8812f36c3491342d6" +checksum = "85754d98985841b7d4f5e8e6fbfa4a4ac847916893ec511a2917ccd8525b8bb3" [[package]] name = "futures-task" -version = "0.3.12" +version = "0.3.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "13de07eb8ea81ae445aca7b69f5f7bf15d7bf4912d8ca37d6645c77ae8a58d86" -dependencies = [ - "once_cell", -] +checksum = "fa189ef211c15ee602667a6fcfe1c1fd9e07d42250d2156382820fba33c9df80" [[package]] name = "futures-test" -version = "0.3.12" +version = "0.3.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8b30f48f6b9cd26d8739965d6e3345c511718884fb223795b80dc71d24a9ea9a" +checksum = "f1fe5e51002528907757d5f1648101086f7197f792112db43ba23b06b09e6bce" dependencies = [ "futures-core", "futures-executor", @@ -1220,16 +1219,15 @@ dependencies = [ "futures-sink", "futures-task", "futures-util", - "once_cell", "pin-project 1.0.5", "pin-utils", ] [[package]] name = "futures-util" -version = "0.3.12" +version = "0.3.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "632a8cd0f2a4b3fdea1657f08bde063848c3bd00f9bbf6e256b8be78802e624b" +checksum = "1812c7ab8aedf8d6f2701a43e1243acdbcc2b36ab26e2ad421eb99ac963d96d1" dependencies = [ "futures-channel", "futures-core", @@ -1259,6 +1257,19 @@ dependencies = [ "tonic-build", ] +[[package]] +name = "generator" +version = "0.6.24" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a9fed24fd1e18827652b4d55652899a1e9da8e54d91624dc3437a5bc3a9f9a9c" +dependencies = [ + "cc", + "libc", + "log", + "rustversion", + "winapi", +] + [[package]] name = "generic-array" version = "0.14.4" @@ -1493,9 +1504,9 @@ dependencies = [ [[package]] name = "idna" -version = "0.2.1" +version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "de910d521f7cc3135c4de8db1cb910e0b5ed1dc6f57c381cd07e8e661ce10094" +checksum = "89829a5d69c23d348314a7ac337fe39173b61149a9864deabd260983aed48c21" dependencies = [ "matches", "unicode-bidi", @@ -1754,9 +1765,9 @@ checksum = "b7282d924be3275cec7f6756ff4121987bc6481325397dde6ba3e7802b1a8b1c" [[package]] name = "libloading" -version = "0.6.7" +version = "0.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "351a32417a12d5f7e82c368a66781e307834dae04c6ce0cd4456d52989229883" +checksum = "6f84d96438c15fcd6c3f244c8fce01d1e2b9c6b5623e9c711dc9286d8fc92d6a" dependencies = [ "cfg-if 1.0.0", "winapi", @@ -1803,6 +1814,17 @@ dependencies = [ "tracing-subscriber", ] +[[package]] +name = "loom" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d44c73b4636e497b4917eb21c33539efa3816741a2d3ff26c6316f1b529481a4" +dependencies = [ + "cfg-if 1.0.0", + "generator", + "scoped-tls", +] + [[package]] name = "lz4" version = "1.23.2" @@ -1919,9 +1941,9 @@ dependencies = [ [[package]] name = "mio" -version = "0.7.7" +version = "0.7.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e50ae3f04d169fcc9bde0b547d1c205219b7157e07ded9c5aff03e0637cb3ed7" +checksum = "a5dede4e2065b3842b8b0af444119f3aa331cc7cc2dd20388bfb0f5d5a38823a" dependencies = [ "libc", "log", @@ -2190,9 +2212,9 @@ dependencies = [ [[package]] name = "once_cell" -version = "1.5.2" +version = "1.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "13bd41f508810a131401606d54ac32a467c97172d74ba7662562ebba5ad07fa0" +checksum = "10acf907b94fc1b1a152d08ef97e7759650268cf986bf127f387e602b02c7e5a" dependencies = [ "parking_lot", ] @@ -2351,7 +2373,7 @@ dependencies = [ [[package]] name = "parquet" version = "4.0.0-SNAPSHOT" -source = "git+https://github.com/apache/arrow.git?rev=ad4504e8e85eb8e5babe0f01ca8cf9947499fc40#ad4504e8e85eb8e5babe0f01ca8cf9947499fc40" +source = "git+https://github.com/apache/arrow.git?rev=b5ac048c75cc55f4039d279f554920be3112d7cd#b5ac048c75cc55f4039d279f554920be3112d7cd" dependencies = [ "arrow", "base64 0.12.3", @@ -2798,7 +2820,7 @@ checksum = "9ab346ac5921dc62ffa9f89b7a773907511cdfa5490c572ae9be1be33e8afa4a" dependencies = [ "crossbeam-channel 0.5.0", "crossbeam-deque 0.8.0", - "crossbeam-utils 0.8.1", + "crossbeam-utils 0.8.2", "lazy_static", "num_cpus", ] @@ -2896,9 +2918,9 @@ dependencies = [ [[package]] name = "reqwest" -version = "0.11.0" +version = "0.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fd281b1030aa675fb90aa994d07187645bb3c8fc756ca766e7c3070b439de9de" +checksum = "0460542b551950620a3648c6aa23318ac6b3cd779114bd873209e6e8b5eb1c34" dependencies = [ "async-compression", "base64 0.13.0", @@ -3053,7 +3075,7 @@ dependencies = [ "base64 0.13.0", "blake2b_simd", "constant_time_eq", - "crossbeam-utils 0.8.1", + "crossbeam-utils 0.8.2", ] [[package]] @@ -3102,6 +3124,12 @@ dependencies = [ "security-framework", ] +[[package]] +name = "rustversion" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cb5d2a036dc6d2d8fd16fde3498b04306e29bd193bf306a57427019b823d5acd" + [[package]] name = "rustyline" version = "7.1.0" @@ -3148,6 +3176,12 @@ dependencies = [ "winapi", ] +[[package]] +name = "scoped-tls" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ea6a9290e3c9cf0f18145ef7ffa62d68ee0bf5fcd651017e586dc7fd5da448c2" + [[package]] name = "scopeguard" version = "1.1.0" @@ -3166,9 +3200,9 @@ dependencies = [ [[package]] name = "security-framework" -version = "2.0.0" +version = "2.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c1759c2e3c8580017a484a7ac56d3abc5a6c1feadf88db2f3633f12ae4268c69" +checksum = "c6af1b6204f89cf0069736daf8b852573e3bc34898eee600e95d3dd855c12e81" dependencies = [ "bitflags", "core-foundation", @@ -3179,9 +3213,9 @@ dependencies = [ [[package]] name = "security-framework-sys" -version = "2.0.0" +version = "2.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f99b9d5e26d2a71633cc4f2ebae7cc9f874044e0c351a27e17892d76dce5678b" +checksum = "31531d257baab426203cf81c5ce1b0b55159dda7ed602ac81b582ccd62265741" dependencies = [ "core-foundation-sys", "libc", @@ -3633,18 +3667,18 @@ dependencies = [ [[package]] name = "thiserror" -version = "1.0.23" +version = "1.0.24" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "76cc616c6abf8c8928e2fdcc0dbfab37175edd8fb49a4641066ad1364fdab146" +checksum = "e0f4a65597094d4483ddaed134f409b2cb7c1beccf25201a9f73c719254fa98e" dependencies = [ "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "1.0.23" +version = "1.0.24" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9be73a2caec27583d0046ef3796c3794f868a5bc813db689eed00c7631275cd1" +checksum = "7765189610d8241a44529806d6fd1f2e0a08734313a35d5b3a556f92b381f3c0" dependencies = [ "proc-macro2", "quote", @@ -3907,9 +3941,9 @@ checksum = "360dfd1d6d30e05fda32ace2c8c70e9c0a9da713275777f5a4dbb8a1893930c6" [[package]] name = "tracing" -version = "0.1.23" +version = "0.1.25" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f7d40a22fd029e33300d8d89a5cc8ffce18bb7c587662f54629e94c9de5487f3" +checksum = "01ebdc2bb4498ab1ab5f5b73c5803825e60199229ccba0698170e3be0e7f959f" dependencies = [ "cfg-if 1.0.0", "log", @@ -3920,9 +3954,9 @@ dependencies = [ [[package]] name = "tracing-attributes" -version = "0.1.12" +version = "0.1.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "43f080ea7e4107844ef4766459426fa2d5c1ada2e47edba05dc7fa99d9629f47" +checksum = "a8a9bd1db7706f2373a190b0d067146caa39350c486f3d455b0e33b431f94c07" dependencies = [ "proc-macro2", "quote", @@ -3940,19 +3974,19 @@ dependencies = [ [[package]] name = "tracing-futures" -version = "0.2.4" +version = "0.2.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ab7bb6f14721aa00656086e9335d363c5c8747bae02ebe32ea2c7dece5689b4c" +checksum = "97d095ae15e245a057c8e8451bab9b3ee1e1f68e9ba2b4fbc18d0ac5237835f2" dependencies = [ - "pin-project 0.4.27", + "pin-project 1.0.5", "tracing", ] [[package]] name = "tracing-log" -version = "0.1.1" +version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5e0f8c7178e13481ff6765bd169b33e8d554c5d2bbede5e32c356194be02b9b9" +checksum = "a6923477a48e41c1951f1999ef8bb5a3023eb723ceadafe78ffb65dc366761e3" dependencies = [ "lazy_static", "log", @@ -3984,9 +4018,9 @@ dependencies = [ [[package]] name = "tracing-subscriber" -version = "0.2.15" +version = "0.2.16" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a1fa8f0c8f4c594e4fc9debc1990deab13238077271ba84dd853d54902ee3401" +checksum = "8ab8966ac3ca27126141f7999361cc97dd6fb4b71da04c02044fa9045d98bb96" dependencies = [ "ansi_term 0.12.1", "chrono", @@ -4067,9 +4101,9 @@ checksum = "a156c684c91ea7d62626509bce3cb4e1d9ed5c4d978f7b4352658f96a4c26b4a" [[package]] name = "url" -version = "2.2.0" +version = "2.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5909f2b0817350449ed73e8bcd81c8c3c8d9a7a5d8acba4b27db277f1868976e" +checksum = "9ccd964113622c8e9322cfac19eb1004a07e636c545f325da085d5cdde6f1f8b" dependencies = [ "form_urlencoded", "idna", diff --git a/arrow_deps/Cargo.toml b/arrow_deps/Cargo.toml index d2e9d9719d..1900c584e0 100644 --- a/arrow_deps/Cargo.toml +++ b/arrow_deps/Cargo.toml @@ -8,11 +8,11 @@ description = "Apache Arrow / Parquet / DataFusion dependencies for InfluxDB IOx [dependencies] # In alphabetical order # We are using development version of arrow/parquet/datafusion and the dependencies are at the same rev -# The version can be found here: https://github.com/apache/arrow/commit/ad4504e8e85eb8e5babe0f01ca8cf9947499fc40 +# The version can be found here: https://github.com/apache/arrow/commit/b5ac048c75cc55f4039d279f554920be3112d7cd # -arrow = { git = "https://github.com/apache/arrow.git", rev = "ad4504e8e85eb8e5babe0f01ca8cf9947499fc40" , features = ["simd"] } -arrow-flight = { git = "https://github.com/apache/arrow.git", rev = "ad4504e8e85eb8e5babe0f01ca8cf9947499fc40" } -datafusion = { git = "https://github.com/apache/arrow.git", rev = "ad4504e8e85eb8e5babe0f01ca8cf9947499fc40" } +arrow = { git = "https://github.com/apache/arrow.git", rev = "b5ac048c75cc55f4039d279f554920be3112d7cd" , features = ["simd"] } +arrow-flight = { git = "https://github.com/apache/arrow.git", rev = "b5ac048c75cc55f4039d279f554920be3112d7cd" } +datafusion = { git = "https://github.com/apache/arrow.git", rev = "b5ac048c75cc55f4039d279f554920be3112d7cd" } # Turn off the "arrow" feature; it currently has a bug that causes the crate to rebuild every time # and we're not currently using it anyway -parquet = { git = "https://github.com/apache/arrow.git", rev = "ad4504e8e85eb8e5babe0f01ca8cf9947499fc40", default-features = false, features = ["snap", "brotli", "flate2", "lz4", "zstd"] } +parquet = { git = "https://github.com/apache/arrow.git", rev = "b5ac048c75cc55f4039d279f554920be3112d7cd", default-features = false, features = ["snap", "brotli", "flate2", "lz4", "zstd"] } From ffc20fa8219dfd0769a8bb69e438c466e9d2e26b Mon Sep 17 00:00:00 2001 From: Raphael Taylor-Davies <1781103+tustvold@users.noreply.github.com> Date: Thu, 25 Feb 2021 13:24:12 +0000 Subject: [PATCH 12/22] feat: add basic gRPC health service (#862) * feat: add basic gRPC health service * feat: update README.md add /health to HTTP API * feat: add health client to influxdb_iox_client feat: end-to-end test health check service Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --- Cargo.lock | 17 +++++ Cargo.toml | 1 + README.md | 16 +++++ buf.yaml | 1 + generated_types/build.rs | 2 + .../protos/grpc/health/v1/service.proto | 23 ++++++ generated_types/src/lib.rs | 18 ++++- influxdb_iox_client/Cargo.toml | 5 +- influxdb_iox_client/src/client.rs | 3 + influxdb_iox_client/src/client/health.rs | 70 +++++++++++++++++++ src/influxdb_ioxd/http.rs | 25 ++++++- src/influxdb_ioxd/rpc.rs | 15 ++++ tests/end-to-end.rs | 25 ++++++- 13 files changed, 215 insertions(+), 6 deletions(-) create mode 100644 generated_types/protos/grpc/health/v1/service.proto create mode 100644 influxdb_iox_client/src/client/health.rs diff --git a/Cargo.lock b/Cargo.lock index 6345cf0326..f1c94743d9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1579,6 +1579,7 @@ dependencies = [ "tokio", "tokio-stream", "tonic", + "tonic-health", "tracing", "tracing-futures", "tracing-opentelemetry", @@ -1593,6 +1594,7 @@ dependencies = [ "arrow_deps", "data_types", "futures-util", + "generated_types", "rand 0.8.3", "reqwest", "serde", @@ -3873,6 +3875,21 @@ dependencies = [ "syn", ] +[[package]] +name = "tonic-health" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a93d6649c8f5436d65337af08887a516183a096d785ef1fc3acf69ed60dbec6b" +dependencies = [ + "async-stream", + "bytes", + "prost", + "tokio", + "tokio-stream", + "tonic", + "tonic-build", +] + [[package]] name = "tower" version = "0.4.5" diff --git a/Cargo.toml b/Cargo.toml index 804db184a7..5a050db810 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -81,6 +81,7 @@ structopt = "0.3.21" tokio = { version = "1.0", features = ["macros", "rt-multi-thread", "parking_lot"] } tokio-stream = { version = "0.1.2", features = ["net"] } tonic = "0.4.0" +tonic-health = "0.3.0" tracing = { version = "0.1", features = ["release_max_level_debug"] } tracing-futures = "0.2.4" tracing-opentelemetry = "0.11.0" diff --git a/README.md b/README.md index b14a5afabf..f4736ece48 100644 --- a/README.md +++ b/README.md @@ -196,6 +196,22 @@ all data in the `company` organization's `sensors` bucket for the `processes` me curl -v -G -d 'org=company' -d 'bucket=sensors' --data-urlencode 'sql_query=select * from processes' "http://127.0.0.1:8080/api/v2/read" ``` +### Health Checks + +The HTTP API exposes a healthcheck endpoint at `/health` + +```shell +$ curl http://127.0.0.1:8080/health +OK +``` + +The gRPC API implements the [gRPC Health Checking Protocol](https://github.com/grpc/grpc/blob/master/doc/health-checking.md). This can be tested with [grpc-health-probe](https://github.com/grpc-ecosystem/grpc-health-probe) + +```shell +$ grpc_health_probe -addr 127.0.0.1:8082 -service influxdata.platform.storage.Storage +status: SERVING +``` + ## Contributing We welcome community contributions from anyone! diff --git a/buf.yaml b/buf.yaml index 030255f9de..fabedb23ea 100644 --- a/buf.yaml +++ b/buf.yaml @@ -5,6 +5,7 @@ build: excludes: - generated_types/protos/com - generated_types/protos/influxdata/platform + - generated_types/protos/grpc lint: use: diff --git a/generated_types/build.rs b/generated_types/build.rs index b632f70123..57261e1558 100644 --- a/generated_types/build.rs +++ b/generated_types/build.rs @@ -28,6 +28,7 @@ fn generate_grpc_types(root: &Path) -> Result<()> { let storage_path = root.join("influxdata/platform/storage"); let idpe_path = root.join("com/github/influxdata/idpe/storage/read"); let management_path = root.join("influxdata/iox/management/v1"); + let grpc_path = root.join("grpc/health/v1"); let proto_files = vec![ storage_path.join("test.proto"), @@ -39,6 +40,7 @@ fn generate_grpc_types(root: &Path) -> Result<()> { management_path.join("base_types.proto"), management_path.join("database_rules.proto"), management_path.join("service.proto"), + grpc_path.join("service.proto"), ]; // Tell cargo to recompile if any of these proto files are changed diff --git a/generated_types/protos/grpc/health/v1/service.proto b/generated_types/protos/grpc/health/v1/service.proto new file mode 100644 index 0000000000..7be24c77c8 --- /dev/null +++ b/generated_types/protos/grpc/health/v1/service.proto @@ -0,0 +1,23 @@ +syntax = "proto3"; + +package grpc.health.v1; + +message HealthCheckRequest { + string service = 1; +} + +message HealthCheckResponse { + enum ServingStatus { + UNKNOWN = 0; + SERVING = 1; + NOT_SERVING = 2; + SERVICE_UNKNOWN = 3; // Used only by the Watch method. + } + ServingStatus status = 1; +} + +service Health { + rpc Check(HealthCheckRequest) returns (HealthCheckResponse); + + rpc Watch(HealthCheckRequest) returns (stream HealthCheckResponse); +} diff --git a/generated_types/src/lib.rs b/generated_types/src/lib.rs index 487164cad1..0cb21817ce 100644 --- a/generated_types/src/lib.rs +++ b/generated_types/src/lib.rs @@ -52,12 +52,28 @@ mod pb { } } } + + // Needed because of https://github.com/hyperium/tonic/issues/471 + pub mod grpc { + pub mod health { + pub mod v1 { + include!(concat!(env!("OUT_DIR"), "/grpc.health.v1.rs")); + } + } + } } include!(concat!(env!("OUT_DIR"), "/wal_generated.rs")); +/// gRPC Storage Service +pub const STORAGE_SERVICE: &str = "influxdata.platform.storage.Storage"; +/// gRPC Testing Service +pub const IOX_TESTING_SERVICE: &str = "influxdata.platform.storage.IOxTesting"; +/// gRPC Arrow Flight Service +pub const ARROW_SERVICE: &str = "arrow.flight.protocol.FlightService"; + pub use pb::com::github::influxdata::idpe::storage::read::*; pub use pb::influxdata::platform::storage::*; pub use google_types as google; -pub use pb::influxdata; +pub use pb::{grpc, influxdata}; diff --git a/influxdb_iox_client/Cargo.toml b/influxdb_iox_client/Cargo.toml index ff4eaa082b..b854697287 100644 --- a/influxdb_iox_client/Cargo.toml +++ b/influxdb_iox_client/Cargo.toml @@ -5,12 +5,13 @@ authors = ["Dom Dwyer "] edition = "2018" [features] -flight = ["arrow_deps", "serde/derive", "tonic", "serde_json", "futures-util"] +flight = ["arrow_deps", "serde/derive", "serde_json", "futures-util"] [dependencies] # Workspace dependencies, in alphabetical order arrow_deps = { path = "../arrow_deps", optional = true } data_types = { path = "../data_types" } +generated_types = { path = "../generated_types" } # Crates.io dependencies, in alphabetical order futures-util = { version = "0.3.1", optional = true } @@ -19,7 +20,7 @@ serde = "1.0.118" serde_json = { version = "1.0.44", optional = true } thiserror = "1.0.23" tokio = { version = "1.0", features = ["macros"] } -tonic = { version = "0.4.0", optional = true } +tonic = { version = "0.4.0" } [dev-dependencies] # In alphabetical order rand = "0.8.1" diff --git a/influxdb_iox_client/src/client.rs b/influxdb_iox_client/src/client.rs index bb4523481e..d04daad147 100644 --- a/influxdb_iox_client/src/client.rs +++ b/influxdb_iox_client/src/client.rs @@ -9,6 +9,9 @@ use data_types::{http::ListDatabasesResponse, DatabaseName}; #[cfg(feature = "flight")] mod flight; +/// Client for the gRPC health checking API +pub mod health; + // can't combine these into one statement that uses `{}` because of this bug in // the `unreachable_pub` lint: https://github.com/rust-lang/rust/issues/64762 #[cfg(feature = "flight")] diff --git a/influxdb_iox_client/src/client/health.rs b/influxdb_iox_client/src/client/health.rs new file mode 100644 index 0000000000..a376c018e4 --- /dev/null +++ b/influxdb_iox_client/src/client/health.rs @@ -0,0 +1,70 @@ +use generated_types::grpc::health::v1::*; +use thiserror::Error; + +/// Error type for the health check client +#[derive(Debug, Error)] +pub enum Error { + /// Service is not serving + #[error("Service is not serving")] + NotServing, + + /// Service returned an unexpected variant for the status enumeration + #[error("Received invalid response: {}", .0)] + InvalidResponse(i32), + + /// Error connecting to the server + #[error("Connection error: {}", .0)] + ConnectionError(#[from] tonic::transport::Error), + + /// Client received an unexpected error from the server + #[error("Unexpected server error: {}: {}", .0.code(), .0.message())] + UnexpectedError(#[from] tonic::Status), +} + +/// Result type for the health check client +pub type Result = std::result::Result; + +/// A client for the gRPC health checking API +/// +/// Allows checking the status of a given service +#[derive(Debug)] +pub struct Client { + inner: health_client::HealthClient, +} + +impl Client { + /// Create a new client with the provided endpoint + pub async fn connect(dst: D) -> Result + where + D: std::convert::TryInto, + D::Error: Into, + { + Ok(Self { + inner: health_client::HealthClient::connect(dst).await?, + }) + } + + /// Returns `Ok()` if the corresponding service is serving + pub async fn check(&mut self, service: impl Into) -> Result<()> { + use health_check_response::ServingStatus; + + let status = self + .inner + .check(HealthCheckRequest { + service: service.into(), + }) + .await? + .into_inner(); + + match status.status() { + ServingStatus::Serving => Ok(()), + ServingStatus::NotServing => Err(Error::NotServing), + _ => Err(Error::InvalidResponse(status.status)), + } + } + + /// Returns `Ok()` if the storage service is serving + pub async fn check_storage(&mut self) -> Result<()> { + self.check(generated_types::STORAGE_SERVICE).await + } +} diff --git a/src/influxdb_ioxd/http.rs b/src/influxdb_ioxd/http.rs index 07adc1b96b..8402fc2b1b 100644 --- a/src/influxdb_ioxd/http.rs +++ b/src/influxdb_ioxd/http.rs @@ -259,6 +259,7 @@ where })) // this endpoint is for API backward compatibility with InfluxDB 2.x .post("/api/v2/write", write::) .get("/ping", ping) + .get("/health", health) .get("/api/v2/read", read::) .get("/iox/api/v1/databases", list_databases::) .put("/iox/api/v1/databases/:name", create_database::) @@ -686,11 +687,17 @@ async fn get_writer( // Route to test that the server is alive #[tracing::instrument(level = "debug")] -async fn ping(req: Request) -> Result, ApplicationError> { +async fn ping(_: Request) -> Result, ApplicationError> { let response_body = "PONG"; Ok(Response::new(Body::from(response_body.to_string()))) } +#[tracing::instrument(level = "debug")] +async fn health(_: Request) -> Result, ApplicationError> { + let response_body = "OK"; + Ok(Response::new(Body::from(response_body.to_string()))) +} + #[derive(Deserialize, Debug)] /// Arguments in the query string of the request to /partitions struct DatabaseInfo { @@ -832,6 +839,22 @@ mod tests { Ok(()) } + #[tokio::test] + async fn test_health() -> Result<()> { + let test_storage = Arc::new(AppServer::new( + ConnectionManagerImpl {}, + Arc::new(ObjectStore::new_in_memory(InMemory::new())), + )); + let server_url = test_server(Arc::clone(&test_storage)); + + let client = Client::new(); + let response = client.get(&format!("{}/health", server_url)).send().await; + + // Print the response so if the test fails, we have a log of what went wrong + check_response("health", response, StatusCode::OK, "OK").await; + Ok(()) + } + #[tokio::test] async fn test_write() -> Result<()> { let test_storage = Arc::new(AppServer::new( diff --git a/src/influxdb_ioxd/rpc.rs b/src/influxdb_ioxd/rpc.rs index d67b0a0382..3f85a2a0e4 100644 --- a/src/influxdb_ioxd/rpc.rs +++ b/src/influxdb_ioxd/rpc.rs @@ -30,7 +30,22 @@ where { let stream = TcpListenerStream::new(socket); + let (mut health_reporter, health_service) = tonic_health::server::health_reporter(); + + let services = [ + generated_types::STORAGE_SERVICE, + generated_types::IOX_TESTING_SERVICE, + generated_types::ARROW_SERVICE, + ]; + + for service in &services { + health_reporter + .set_service_status(service, tonic_health::ServingStatus::Serving) + .await; + } + tonic::transport::Server::builder() + .add_service(health_service) .add_service(testing::make_server()) .add_service(storage::make_server(Arc::clone(&server))) .add_service(flight::make_server(server)) diff --git a/tests/end-to-end.rs b/tests/end-to-end.rs index e6530709b6..76f45430eb 100644 --- a/tests/end-to-end.rs +++ b/tests/end-to-end.rs @@ -377,6 +377,27 @@ impl TestServer { // different ports but both need to be up for the test to run let try_grpc_connect = async { let mut interval = tokio::time::interval(Duration::from_millis(500)); + + loop { + match influxdb_iox_client::health::Client::connect(GRPC_URL_BASE).await { + Ok(mut client) => { + println!("Successfully connected to server"); + + match client.check_storage().await { + Ok(_) => { + println!("Storage service is running"); + break; + } + Err(e) => println!("Error checking storage service status: {}", e), + } + } + Err(e) => { + println!("Waiting for gRPC API to be up: {}", e); + } + } + interval.tick().await; + } + loop { match StorageClient::connect(GRPC_URL_BASE).await { Ok(storage_client) => { @@ -387,7 +408,7 @@ impl TestServer { return; } Err(e) => { - println!("Waiting for gRPC server to be up: {}", e); + println!("Failed to create storage client: {}", e) } } interval.tick().await; @@ -396,7 +417,7 @@ impl TestServer { let try_http_connect = async { let client = reqwest::Client::new(); - let url = format!("{}/ping", HTTP_BASE); + let url = format!("{}/health", HTTP_BASE); let mut interval = tokio::time::interval(Duration::from_millis(500)); loop { match client.get(&url).send().await { From 96f9395299f435aabd03d5c779f2ba2091b1ca81 Mon Sep 17 00:00:00 2001 From: Raphael Taylor-Davies Date: Thu, 25 Feb 2021 15:05:54 +0000 Subject: [PATCH 13/22] feat: smaller IOx build images --- .circleci/config.yml | 4 ++-- .dockerignore | 4 ++++ docker/{Dockerfile.perf => Dockerfile.iox} | 10 +++++++--- 3 files changed, 13 insertions(+), 5 deletions(-) create mode 100644 .dockerignore rename docker/{Dockerfile.perf => Dockerfile.iox} (52%) diff --git a/.circleci/config.yml b/.circleci/config.yml index ecc2deaac3..934456c301 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -86,7 +86,7 @@ jobs: # out for parallel CI runs! # # To change the contents of the build container, modify docker/Dockerfile.ci - # To change the final release container, modify docker/Dockerfile.perf + # To change the final release container, modify docker/Dockerfile.iox perf_image: docker: - image: quay.io/influxdb/rust:ci @@ -105,7 +105,7 @@ jobs: echo "$QUAY_PASS" | docker login quay.io --username $QUAY_USER --password-stdin - run: | BRANCH=$(git rev-parse --abbrev-ref HEAD | tr '/' '.') - docker build -t quay.io/influxdb/fusion:$BRANCH -f docker/Dockerfile.perf . + docker build -t quay.io/influxdb/fusion:$BRANCH -f docker/Dockerfile.iox . docker push quay.io/influxdb/fusion:$BRANCH echo "export BRANCH=${BRANCH}" >> $BASH_ENV - run: diff --git a/.dockerignore b/.dockerignore new file mode 100644 index 0000000000..2b8fb97f19 --- /dev/null +++ b/.dockerignore @@ -0,0 +1,4 @@ +# Ignore everything +** +# Except +!target/release/influxdb_iox \ No newline at end of file diff --git a/docker/Dockerfile.perf b/docker/Dockerfile.iox similarity index 52% rename from docker/Dockerfile.perf rename to docker/Dockerfile.iox index b4834888b0..e62d8f396d 100644 --- a/docker/Dockerfile.perf +++ b/docker/Dockerfile.iox @@ -1,7 +1,11 @@ ### -# Dockerfile for the image used in CI performance tests +# Dockerfile used for deploying IOx ## -FROM rust:slim-buster +FROM debian:buster-slim + +RUN apt-get update \ + && apt-get install -y libssl1.1 libgcc1 libc6 \ + && rm -rf /var/lib/{apt,dpkg,cache,log} RUN groupadd -g 1500 rust \ && useradd -u 1500 -g rust -s /bin/bash -m rust @@ -15,4 +19,4 @@ COPY target/release/influxdb_iox /usr/bin/influxdb_iox EXPOSE 8080 8082 -CMD ["influxdb_iox"] +ENTRYPOINT ["influxdb_iox"] From c7ef18337cdfe20d96934590b91eb81a1f4ea4ba Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Wed, 24 Feb 2021 16:52:18 -0500 Subject: [PATCH 14/22] feat: Consolidate all bucket config into one option/env var Fixes #869. --- docs/README.md | 2 ++ docs/env.example | 6 ++-- docs/testing.md | 27 +++++++++++++++ object_store/src/aws.rs | 14 ++++---- object_store/src/azure.rs | 6 ++-- object_store/src/gcp.rs | 10 +++--- src/commands/config.rs | 44 ++++++++++++++++++++---- src/influxdb_ioxd.rs | 71 +++++++++++++++++++++++++++++---------- 8 files changed, 139 insertions(+), 41 deletions(-) create mode 100644 docs/testing.md diff --git a/docs/README.md b/docs/README.md index dd32a28e41..a046bf8a60 100644 --- a/docs/README.md +++ b/docs/README.md @@ -5,9 +5,11 @@ interest for those who wish to understand how the code works. It is not intended to be general user facing documentation ## Table of Contents: + * Rust style and Idiom guide: [style_guide.md](style_guide.md) * Tracing and logging Guide: [tracing.md](tracing.md) * How InfluxDB IOx manages the lifecycle of time series data: [data_management.md](data_management.md) * Thoughts on parquet encoding and compression for timeseries data: [encoding_thoughts.md](encoding_thoughts.md) * Thoughts on using multiple cores: [multi_core_tasks.md](multi_core_tasks.md) * [Query Engine Docs](../query/README.md) +* [Testing documentation](testing.md) for developers of IOx diff --git a/docs/env.example b/docs/env.example index bd4a27eeae..6bcf5a429e 100644 --- a/docs/env.example +++ b/docs/env.example @@ -28,10 +28,10 @@ # AWS_ACCESS_KEY_ID=access_key_value # AWS_SECRET_ACCESS_KEY=secret_access_key_value # AWS_DEFAULT_REGION=us-east-2 -# INFLUXDB_IOX_S3_BUCKET=bucket-name +# INFLUXDB_IOX_BUCKET=bucket-name # # If using Google Cloud Storage as an object store: -# INFLUXDB_IOX_GCP_BUCKET=bucket_name +# INFLUXDB_IOX_BUCKET=bucket_name # Set one of SERVICE_ACCOUNT or GOOGLE_APPLICATION_CREDENTIALS, either to a path of a filename # containing Google credential JSON or to the JSON directly. # SERVICE_ACCOUNT=/path/to/auth/info.json @@ -41,7 +41,7 @@ # The name you see when going to All Services > Storage accounts > [name] # AZURE_STORAGE_ACCOUNT= # The name of a container you've created in the storage account, under Blob Service > Containers -# AZURE_STORAGE_CONTAINER= +# INFLUXDB_IOX_BUCKET= # In the Storage account's Settings > Access keys, one of the Key values # AZURE_STORAGE_MASTER_KEY= # diff --git a/docs/testing.md b/docs/testing.md new file mode 100644 index 0000000000..0e7448e28d --- /dev/null +++ b/docs/testing.md @@ -0,0 +1,27 @@ +# Testing + +This document covers details that are only relevant if you are developing IOx and running the tests. + +## Object storage + +### To run the tests or not run the tests + +If you are testing integration with some or all of the object storage options, you'll have more +setup to do. + +By default, `cargo test --workspace` does not run any tests that actually contact any cloud +services: tests that do contact the services will silently pass. + +To ensure you've configured object storage integration testing correctly, you can run +`TEST_INTEGRATION=1 cargo test --workspace`, which will run the tests that contact the cloud +services and fail them if the required environment variables aren't set. + +If you don't specify the `TEST_INTEGRATION` environment variable but you do configure some or all +of the object stores, the relevant tests will run. + +### Configuration differences when running the tests + +When running `influxdb_iox server`, you can pick one object store to use. When running the tests, +you can run them against all the possible object stores. There's still only one +`INFLUXDB_IOX_BUCKET` variable, though, so that will set the bucket name for all configured object +stores. Use the same bucket name when setting up the different services. diff --git a/object_store/src/aws.rs b/object_store/src/aws.rs index daf2243d44..b982ac4e68 100644 --- a/object_store/src/aws.rs +++ b/object_store/src/aws.rs @@ -419,26 +419,26 @@ mod tests { dotenv::dotenv().ok(); let region = env::var("AWS_DEFAULT_REGION"); - let bucket_name = env::var("INFLUXDB_IOX_S3_BUCKET"); + let bucket_name = env::var("INFLUXDB_IOX_BUCKET"); let force = std::env::var("TEST_INTEGRATION"); match (region.is_ok(), bucket_name.is_ok(), force.is_ok()) { (false, false, true) => { panic!( "TEST_INTEGRATION is set, \ - but AWS_DEFAULT_REGION and INFLUXDB_IOX_S3_BUCKET are not" + but AWS_DEFAULT_REGION and INFLUXDB_IOX_BUCKET are not" ) } (false, true, true) => { panic!("TEST_INTEGRATION is set, but AWS_DEFAULT_REGION is not") } (true, false, true) => { - panic!("TEST_INTEGRATION is set, but INFLUXDB_IOX_S3_BUCKET is not") + panic!("TEST_INTEGRATION is set, but INFLUXDB_IOX_BUCKET is not") } (false, false, false) => { eprintln!( "skipping integration test - set \ - AWS_DEFAULT_REGION and INFLUXDB_IOX_S3_BUCKET to run" + AWS_DEFAULT_REGION and INFLUXDB_IOX_BUCKET to run" ); return Ok(()); } @@ -447,7 +447,7 @@ mod tests { return Ok(()); } (true, false, false) => { - eprintln!("skipping integration test - set INFLUXDB_IOX_S3_BUCKET to run"); + eprintln!("skipping integration test - set INFLUXDB_IOX_BUCKET to run"); return Ok(()); } _ => {} @@ -466,8 +466,8 @@ mod tests { "The environment variable AWS_DEFAULT_REGION must be set \ to a value like `us-east-2`" })?; - let bucket_name = env::var("INFLUXDB_IOX_S3_BUCKET") - .map_err(|_| "The environment variable INFLUXDB_IOX_S3_BUCKET must be set")?; + let bucket_name = env::var("INFLUXDB_IOX_BUCKET") + .map_err(|_| "The environment variable INFLUXDB_IOX_BUCKET must be set")?; Ok((region.parse()?, bucket_name)) } diff --git a/object_store/src/azure.rs b/object_store/src/azure.rs index 1455de4e4b..99085a763b 100644 --- a/object_store/src/azure.rs +++ b/object_store/src/azure.rs @@ -299,7 +299,7 @@ mod tests { let required_vars = [ "AZURE_STORAGE_ACCOUNT", - "AZURE_STORAGE_CONTAINER", + "INFLUXDB_IOX_BUCKET", "AZURE_STORAGE_MASTER_KEY", ]; let unset_vars: Vec<_> = required_vars @@ -334,8 +334,8 @@ mod tests { async fn azure_blob_test() -> Result<()> { maybe_skip_integration!(); - let container_name = env::var("AZURE_STORAGE_CONTAINER") - .map_err(|_| "The environment variable AZURE_STORAGE_CONTAINER must be set")?; + let container_name = env::var("INFLUXDB_IOX_BUCKET") + .map_err(|_| "The environment variable INFLUXDB_IOX_BUCKET must be set")?; let integration = MicrosoftAzure::new_from_env(container_name); put_get_delete_list(&integration).await?; diff --git a/object_store/src/gcp.rs b/object_store/src/gcp.rs index ac60170508..8a7021e744 100644 --- a/object_store/src/gcp.rs +++ b/object_store/src/gcp.rs @@ -267,15 +267,15 @@ mod test { () => { dotenv::dotenv().ok(); - let bucket_name = env::var("GCS_BUCKET_NAME"); + let bucket_name = env::var("INFLUXDB_IOX_BUCKET"); let force = std::env::var("TEST_INTEGRATION"); match (bucket_name.is_ok(), force.is_ok()) { (false, true) => { - panic!("TEST_INTEGRATION is set, but GCS_BUCKET_NAME is not") + panic!("TEST_INTEGRATION is set, but INFLUXDB_IOX_BUCKET is not") } (false, false) => { - eprintln!("skipping integration test - set GCS_BUCKET_NAME to run"); + eprintln!("skipping integration test - set INFLUXDB_IOX_BUCKET to run"); return Ok(()); } _ => {} @@ -284,8 +284,8 @@ mod test { } fn bucket_name() -> Result { - Ok(env::var("GCS_BUCKET_NAME") - .map_err(|_| "The environment variable GCS_BUCKET_NAME must be set")?) + Ok(env::var("INFLUXDB_IOX_BUCKET") + .map_err(|_| "The environment variable INFLUXDB_IOX_BUCKET must be set")?) } #[tokio::test] diff --git a/src/commands/config.rs b/src/commands/config.rs index 0840038004..a4fb322440 100644 --- a/src/commands/config.rs +++ b/src/commands/config.rs @@ -1,8 +1,8 @@ //! Implementation of command line option for manipulating and showing server //! config +use clap::arg_enum; use std::{net::SocketAddr, net::ToSocketAddrs, path::PathBuf}; - use structopt::StructOpt; /// The default bind address for the HTTP API. @@ -91,16 +91,37 @@ pub struct Config { #[structopt(long = "--data-dir", env = "INFLUXDB_IOX_DB_DIR")] pub database_directory: Option, + #[structopt( + long = "--object-store", + env = "INFLUXDB_IOX_OBJECT_STORE", + possible_values = &ObjectStore::variants(), + case_insensitive = true, + long_help = r#"Which object storage to use. If not specified, defaults to memory. + +Possible values (case insensitive): + +* memory (default): Effectively no object persistence. +* file: Stores objects in the local filesystem. Must also set `--database_directory`. +* s3: Amazon S3. Must also set `--bucket`, AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, and + AWS_DEFAULT_REGION. +* google: Google Cloud Storage. Must also set `--bucket` and SERVICE_ACCOUNT. +* azure: Microsoft Azure blob storage. Must also set `--bucket`, AZURE_STORAGE_ACCOUNT, + and AZURE_STORAGE_MASTER_KEY. + "#, + )] + pub object_store: Option, + + /// Name of the bucket to use for the object store. Must also set + /// `--object_store` to a cloud object storage to have any effect. + /// /// If using Google Cloud Storage for the object store, this item, as well /// as SERVICE_ACCOUNT must be set. - #[structopt(long = "--gcp-bucket", env = "INFLUXDB_IOX_GCP_BUCKET")] - pub gcp_bucket: Option, - + /// /// If using S3 for the object store, this item, as well /// as AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY and AWS_DEFAULT_REGION must /// be set. - #[structopt(long = "--s3-bucket", env = "INFLUXDB_IOX_S3_BUCKET")] - pub s3_bucket: Option, + #[structopt(long = "--bucket", env = "INFLUXDB_IOX_BUCKET")] + pub bucket: Option, /// If set, Jaeger traces are emitted to this host /// using the OpenTelemetry tracer. @@ -167,6 +188,17 @@ fn strip_server(args: impl Iterator) -> Vec { .collect::>() } +arg_enum! { + #[derive(Debug, Copy, Clone, PartialEq)] + pub enum ObjectStore { + Memory, + File, + S3, + Google, + Azure, + } +} + /// How to format output logging messages #[derive(Debug, Clone, Copy)] pub enum LogFormat { diff --git a/src/influxdb_ioxd.rs b/src/influxdb_ioxd.rs index 7e0eec849d..10019b7704 100644 --- a/src/influxdb_ioxd.rs +++ b/src/influxdb_ioxd.rs @@ -12,7 +12,7 @@ use panic_logging::SendPanicsToTracing; use server::{ConnectionManagerImpl as ConnectionManager, Server as AppServer}; use crate::commands::{ - config::{load_config, Config}, + config::{load_config, Config, ObjectStore as ObjStoreOpt}, logging::LoggingLevel, }; @@ -64,6 +64,12 @@ pub enum Error { #[snafu(display("Error serving RPC: {}", source))] ServingRPC { source: self::rpc::Error }, + + #[snafu(display("Specifed {} for the object store, but not a bucket", object_store))] + InvalidCloudObjectStoreConfiguration { object_store: ObjStoreOpt }, + + #[snafu(display("Specified file for the object store, but not a database directory"))] + InvalidFileObjectStoreConfiguration, } pub type Result = std::result::Result; @@ -92,22 +98,53 @@ pub async fn main(logging_level: LoggingLevel, config: Option) -> Result let f = SendPanicsToTracing::new(); std::mem::forget(f); - let db_dir = &config.database_directory; - - let object_store = if let Some(bucket_name) = &config.gcp_bucket { - info!("Using GCP bucket {} for storage", bucket_name); - ObjectStore::new_google_cloud_storage(GoogleCloudStorage::new(bucket_name)) - } else if let Some(bucket_name) = &config.s3_bucket { - info!("Using S3 bucket {} for storage", bucket_name); - // rusoto::Region's default takes the value from the AWS_DEFAULT_REGION env var. - ObjectStore::new_amazon_s3(AmazonS3::new(Default::default(), bucket_name)) - } else if let Some(db_dir) = db_dir { - info!("Using local dir {:?} for storage", db_dir); - fs::create_dir_all(db_dir).context(CreatingDatabaseDirectory { path: db_dir })?; - ObjectStore::new_file(object_store::disk::File::new(&db_dir)) - } else { - warn!("NO PERSISTENCE: using memory for object storage"); - ObjectStore::new_in_memory(object_store::memory::InMemory::new()) + let object_store = match ( + config.object_store, + config.bucket, + config.database_directory, + ) { + (Some(ObjStoreOpt::Google), Some(bucket), _) => { + info!("Using GCP bucket {} for storage", bucket); + ObjectStore::new_google_cloud_storage(GoogleCloudStorage::new(bucket)) + } + (Some(ObjStoreOpt::Google), None, _) => { + return InvalidCloudObjectStoreConfiguration { + object_store: ObjStoreOpt::Google, + } + .fail(); + } + (Some(ObjStoreOpt::S3), Some(bucket), _) => { + info!("Using S3 bucket {} for storage", bucket); + // rusoto::Region's default takes the value from the AWS_DEFAULT_REGION env var. + ObjectStore::new_amazon_s3(AmazonS3::new(Default::default(), bucket)) + } + (Some(ObjStoreOpt::S3), None, _) => { + return InvalidCloudObjectStoreConfiguration { + object_store: ObjStoreOpt::S3, + } + .fail(); + } + (Some(ObjStoreOpt::File), _, Some(ref db_dir)) => { + info!("Using local dir {:?} for storage", db_dir); + fs::create_dir_all(db_dir).context(CreatingDatabaseDirectory { path: db_dir })?; + ObjectStore::new_file(object_store::disk::File::new(&db_dir)) + } + (Some(ObjStoreOpt::File), _, None) => { + return InvalidFileObjectStoreConfiguration.fail(); + } + (Some(ObjStoreOpt::Azure), Some(_bucket), _) => { + unimplemented!(); + } + (Some(ObjStoreOpt::Azure), None, _) => { + return InvalidCloudObjectStoreConfiguration { + object_store: ObjStoreOpt::Azure, + } + .fail(); + } + (Some(ObjStoreOpt::Memory), _, _) | (None, _, _) => { + warn!("NO PERSISTENCE: using memory for object storage"); + ObjectStore::new_in_memory(object_store::memory::InMemory::new()) + } }; let object_storage = Arc::new(object_store); From ecfd2b9689e052a05bb2dec6e1d00fc247eefdfc Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Thu, 25 Feb 2021 15:24:41 -0500 Subject: [PATCH 15/22] fix: Use the flag name, not the struct field name --- src/commands/config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/commands/config.rs b/src/commands/config.rs index a4fb322440..b1048ded52 100644 --- a/src/commands/config.rs +++ b/src/commands/config.rs @@ -101,7 +101,7 @@ pub struct Config { Possible values (case insensitive): * memory (default): Effectively no object persistence. -* file: Stores objects in the local filesystem. Must also set `--database_directory`. +* file: Stores objects in the local filesystem. Must also set `--data-dir`. * s3: Amazon S3. Must also set `--bucket`, AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, and AWS_DEFAULT_REGION. * google: Google Cloud Storage. Must also set `--bucket` and SERVICE_ACCOUNT. From ac88bb98f93150a3a1d14b7741eddb9ac15a7c05 Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Thu, 25 Feb 2021 15:52:52 -0500 Subject: [PATCH 16/22] docs: Clarify more about TEST_INTEGRATION in regards to both crates that use it --- docs/testing.md | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/docs/testing.md b/docs/testing.md index 0e7448e28d..30c936dfd9 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -9,11 +9,11 @@ This document covers details that are only relevant if you are developing IOx an If you are testing integration with some or all of the object storage options, you'll have more setup to do. -By default, `cargo test --workspace` does not run any tests that actually contact any cloud -services: tests that do contact the services will silently pass. +By default, `cargo test -p object_store` does not run any tests that actually contact +any cloud services: tests that do contact the services will silently pass. To ensure you've configured object storage integration testing correctly, you can run -`TEST_INTEGRATION=1 cargo test --workspace`, which will run the tests that contact the cloud +`TEST_INTEGRATION=1 cargo test -p object_store`, which will run the tests that contact the cloud services and fail them if the required environment variables aren't set. If you don't specify the `TEST_INTEGRATION` environment variable but you do configure some or all @@ -25,3 +25,26 @@ When running `influxdb_iox server`, you can pick one object store to use. When r you can run them against all the possible object stores. There's still only one `INFLUXDB_IOX_BUCKET` variable, though, so that will set the bucket name for all configured object stores. Use the same bucket name when setting up the different services. + +Other than possibly configuring multiple object stores, configuring the tests to use the object +store services is the same as configuring the server to use an object store service. See the output +of `influxdb_iox server --help` for instructions. + +## InfluxDB IOx Client + +The `influxdb_iox_client` crate might be used by people who are using a managed IOx server. In +other words, they might only use the `influxdb_iox_client` crate and not the rest of the crates in +this workspace. The tests in `influxdb_iox_client` see an IOx server in the same way as IOx servers +see the object store services: sometimes you'll want to run the tests against an actual server, and +sometimes you won't. + +Like in the `object_store` crate, the `influxdb_iox_client` crate's tests use the +`TEST_INTEGRATION` environment variable to enforce running tests that use an actual IOx server. +Running `cargo test -p influxdb_iox_client` will silently pass tests that contact a server. + +Start an IOx server in one terminal and run `TEST_INTEGRATION=1 +TEST_IOX_ENDPOINT=http://127.0.0.1:8080 cargo test -p influxdb_iox_client` in another (where +`http://127.0.0.1:8080` is the address to the IOx HTTP server) to run the client tests against the +server. If you set `TEST_INTEGRATION` but not `TEST_IOX_ENDPOINT`, the integration tests will fail +because of the missed configuration. If you set `TEST_IOX_ENDPOINT` but not `TEST_INTEGRATION`, the +integration tests will be run. From 12deacd8a05c0706bd4454f052dd53ecfeade634 Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Thu, 25 Feb 2021 18:12:39 -0500 Subject: [PATCH 17/22] refactor: move SeriesSetPlans into its own module (#878) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --- mutable_buffer/src/database.rs | 7 +- mutable_buffer/src/table.rs | 3 +- query/src/exec.rs | 92 ++---------------------- query/src/lib.rs | 3 +- query/src/plan.rs | 1 + query/src/plan/seriesset.rs | 86 ++++++++++++++++++++++ query/src/test.rs | 7 +- server/src/db.rs | 4 +- src/influxdb_ioxd/rpc/storage/service.rs | 2 +- 9 files changed, 104 insertions(+), 101 deletions(-) create mode 100644 query/src/plan/seriesset.rs diff --git a/mutable_buffer/src/database.rs b/mutable_buffer/src/database.rs index 6641faeb9f..31994698f6 100644 --- a/mutable_buffer/src/database.rs +++ b/mutable_buffer/src/database.rs @@ -3,14 +3,13 @@ use data_types::{ database_rules::{PartitionSort, PartitionSortRules}, }; use generated_types::wal; -use query::group_by::Aggregate; use query::group_by::GroupByAndAggregate; use query::group_by::WindowDuration; use query::{ - exec::{SeriesSetPlan, SeriesSetPlans}, - predicate::Predicate, - Database, + group_by::Aggregate, + plan::seriesset::{SeriesSetPlan, SeriesSetPlans}, }; +use query::{predicate::Predicate, Database}; use crate::column::Column; use crate::table::Table; diff --git a/mutable_buffer/src/table.rs b/mutable_buffer/src/table.rs index ae54691ce9..968ca7e48a 100644 --- a/mutable_buffer/src/table.rs +++ b/mutable_buffer/src/table.rs @@ -1,9 +1,10 @@ use generated_types::wal as wb; use query::{ - exec::{field::FieldColumns, SeriesSetPlan}, + exec::field::FieldColumns, func::selectors::{selector_first, selector_last, selector_max, selector_min, SelectorOutput}, func::window::make_window_bound_expr, group_by::{Aggregate, WindowDuration}, + plan::seriesset::SeriesSetPlan, }; use std::{ diff --git a/query/src/exec.rs b/query/src/exec.rs index 404503b294..4308479e50 100644 --- a/query/src/exec.rs +++ b/query/src/exec.rs @@ -18,7 +18,6 @@ use arrow_deps::{ use counters::ExecutionCounters; use context::IOxExecutionContext; -use field::FieldColumns; use schema_pivot::SchemaPivotNode; use fieldlist::{FieldList, IntoFieldList}; @@ -28,7 +27,11 @@ use tokio::sync::mpsc::{self, error::SendError}; use snafu::{ResultExt, Snafu}; -use crate::plan::{fieldlist::FieldListPlan, stringset::StringSetPlan}; +use crate::plan::{ + fieldlist::FieldListPlan, + seriesset::{SeriesSetPlan, SeriesSetPlans}, + stringset::StringSetPlan, +}; #[derive(Debug, Snafu)] pub enum Error { @@ -85,91 +88,6 @@ pub enum Error { pub type Result = std::result::Result; -/// A plan that can be run to produce a logical stream of time series, -/// as represented as sequence of SeriesSets from a single DataFusion -/// plan, optionally grouped in some way. -#[derive(Debug)] -pub struct SeriesSetPlan { - /// The table name this came from - pub table_name: Arc, - - /// Datafusion plan to execute. The plan must produce - /// RecordBatches that have: - /// - /// * fields for each name in `tag_columns` and `field_columns` - /// * a timestamp column called 'time' - /// * each column in tag_columns must be a String (Utf8) - pub plan: LogicalPlan, - - /// The names of the columns that define tags. - /// - /// Note these are `Arc` strings because they are duplicated for - /// *each* resulting `SeriesSet` that is produced when this type - /// of plan is executed. - pub tag_columns: Vec>, - - /// The names of the columns which are "fields" - /// - /// Note these are `Arc` strings because they are duplicated for - /// *each* resulting `SeriesSet` that is produced when this type - /// of plan is executed. - pub field_columns: FieldColumns, - - /// If present, how many of the series_set_plan::tag_columns - /// should be used to compute the group - pub num_prefix_tag_group_columns: Option, -} - -impl SeriesSetPlan { - /// Create a SeriesSetPlan that will not produce any Group items - pub fn new_from_shared_timestamp( - table_name: Arc, - plan: LogicalPlan, - tag_columns: Vec>, - field_columns: Vec>, - ) -> Self { - Self::new(table_name, plan, tag_columns, field_columns.into()) - } - - /// Create a SeriesSetPlan that will not produce any Group items - pub fn new( - table_name: Arc, - plan: LogicalPlan, - tag_columns: Vec>, - field_columns: FieldColumns, - ) -> Self { - let num_prefix_tag_group_columns = None; - - Self { - table_name, - plan, - tag_columns, - field_columns, - num_prefix_tag_group_columns, - } - } - - /// Create a SeriesSetPlan that will produce Group items, according to - /// num_prefix_tag_group_columns. - pub fn grouped(mut self, num_prefix_tag_group_columns: usize) -> Self { - self.num_prefix_tag_group_columns = Some(num_prefix_tag_group_columns); - self - } -} - -/// A container for plans which each produce a logical stream of -/// timeseries (from across many potential tables). -#[derive(Debug, Default)] -pub struct SeriesSetPlans { - pub plans: Vec, -} - -impl From> for SeriesSetPlans { - fn from(plans: Vec) -> Self { - Self { plans } - } -} - /// Handles executing plans, and marshalling the results into rust /// native structures. #[derive(Debug, Default)] diff --git a/query/src/lib.rs b/query/src/lib.rs index 9ab66c9155..7d2486eda6 100644 --- a/query/src/lib.rs +++ b/query/src/lib.rs @@ -11,7 +11,8 @@ use async_trait::async_trait; use data_types::{ data::ReplicatedWrite, partition_metadata::TableSummary, schema::Schema, selection::Selection, }; -use exec::{stringset::StringSet, Executor, SeriesSetPlans}; +use exec::{stringset::StringSet, Executor}; +use plan::seriesset::SeriesSetPlans; use std::{fmt::Debug, sync::Arc}; diff --git a/query/src/plan.rs b/query/src/plan.rs index b7baba6ffb..693ff90cdc 100644 --- a/query/src/plan.rs +++ b/query/src/plan.rs @@ -1,2 +1,3 @@ pub mod fieldlist; +pub mod seriesset; pub mod stringset; diff --git a/query/src/plan/seriesset.rs b/query/src/plan/seriesset.rs new file mode 100644 index 0000000000..325a87fd36 --- /dev/null +++ b/query/src/plan/seriesset.rs @@ -0,0 +1,86 @@ +use std::sync::Arc; + +use arrow_deps::datafusion::logical_plan::LogicalPlan; + +use crate::exec::field::FieldColumns; + +/// A plan that can be run to produce a logical stream of time series, +/// as represented as sequence of SeriesSets from a single DataFusion +/// plan, optionally grouped in some way. +#[derive(Debug)] +pub struct SeriesSetPlan { + /// The table name this came from + pub table_name: Arc, + + /// Datafusion plan to execute. The plan must produce + /// RecordBatches that have: + /// + /// * fields for each name in `tag_columns` and `field_columns` + /// * a timestamp column called 'time' + /// * each column in tag_columns must be a String (Utf8) + pub plan: LogicalPlan, + + /// The names of the columns that define tags. + /// + /// Note these are `Arc` strings because they are duplicated for + /// *each* resulting `SeriesSet` that is produced when this type + /// of plan is executed. + pub tag_columns: Vec>, + + /// The names of the columns which are "fields" + pub field_columns: FieldColumns, + + /// If present, how many of the series_set_plan::tag_columns + /// should be used to compute the group + pub num_prefix_tag_group_columns: Option, +} + +impl SeriesSetPlan { + /// Create a SeriesSetPlan that will not produce any Group items + pub fn new_from_shared_timestamp( + table_name: Arc, + plan: LogicalPlan, + tag_columns: Vec>, + field_columns: Vec>, + ) -> Self { + Self::new(table_name, plan, tag_columns, field_columns.into()) + } + + /// Create a SeriesSetPlan that will not produce any Group items + pub fn new( + table_name: Arc, + plan: LogicalPlan, + tag_columns: Vec>, + field_columns: FieldColumns, + ) -> Self { + let num_prefix_tag_group_columns = None; + + Self { + table_name, + plan, + tag_columns, + field_columns, + num_prefix_tag_group_columns, + } + } + + /// Create a SeriesSetPlan that will produce Group items, according to + /// num_prefix_tag_group_columns. + pub fn grouped(mut self, num_prefix_tag_group_columns: usize) -> Self { + self.num_prefix_tag_group_columns = Some(num_prefix_tag_group_columns); + self + } +} + +/// A container for plans which each produce a logical stream of +/// timeseries (from across many potential tables). +#[derive(Debug, Default)] +pub struct SeriesSetPlans { + pub plans: Vec, +} + +impl From> for SeriesSetPlans { + fn from(plans: Vec) -> Self { + Self { plans } + } +} diff --git a/query/src/test.rs b/query/src/test.rs index 0329c5c53f..f49d2f8d02 100644 --- a/query/src/test.rs +++ b/query/src/test.rs @@ -12,14 +12,11 @@ use arrow_deps::{ datafusion::physical_plan::{common::SizedRecordBatchStream, SendableRecordBatchStream}, }; -use crate::{exec::Executor, group_by::GroupByAndAggregate}; use crate::{ - exec::{ - stringset::{StringSet, StringSetRef}, - SeriesSetPlans, - }, + exec::stringset::{StringSet, StringSetRef}, Database, DatabaseStore, PartitionChunk, Predicate, }; +use crate::{exec::Executor, group_by::GroupByAndAggregate, plan::seriesset::SeriesSetPlans}; use data_types::{ data::{lines_to_replicated_write, ReplicatedWrite}, diff --git a/server/src/db.rs b/server/src/db.rs index c8ca7cf150..a85d571c71 100644 --- a/server/src/db.rs +++ b/server/src/db.rs @@ -309,7 +309,7 @@ impl Database for Db { async fn query_series( &self, predicate: query::predicate::Predicate, - ) -> Result { + ) -> Result { self.mutable_buffer .as_ref() .context(DatabaseNotReadable)? @@ -322,7 +322,7 @@ impl Database for Db { &self, predicate: query::predicate::Predicate, gby_agg: query::group_by::GroupByAndAggregate, - ) -> Result { + ) -> Result { self.mutable_buffer .as_ref() .context(DatabaseNotReadable)? diff --git a/src/influxdb_ioxd/rpc/storage/service.rs b/src/influxdb_ioxd/rpc/storage/service.rs index f36f85fa38..1cc51fec19 100644 --- a/src/influxdb_ioxd/rpc/storage/service.rs +++ b/src/influxdb_ioxd/rpc/storage/service.rs @@ -1109,8 +1109,8 @@ mod tests { use arrow_deps::datafusion::logical_plan::{col, lit, Expr}; use panic_logging::SendPanicsToTracing; use query::{ - exec::SeriesSetPlans, group_by::{Aggregate as QueryAggregate, WindowDuration as QueryWindowDuration}, + plan::seriesset::SeriesSetPlans, test::QueryGroupsRequest, test::TestDatabaseStore, test::{QuerySeriesRequest, TestChunk}, From b5f2bd27d101010a023960aec5aaecd08bf95299 Mon Sep 17 00:00:00 2001 From: Jacob Marble Date: Tue, 23 Feb 2021 15:32:12 -0800 Subject: [PATCH 18/22] chore: improve readme Add admin calls to set writer ID and create a database. --- README.md | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/README.md b/README.md index f4736ece48..ca16f66e5c 100644 --- a/README.md +++ b/README.md @@ -176,6 +176,31 @@ The server will, by default, start an HTTP API server on port `8080` and a gRPC ### Writing and Reading Data +Each IOx instance requires a writer ID. +This can be set three ways: +- set an environment variable `INFLUXDB_IOX_ID=42` +- set a flag `--writer-id 42` +- send an HTTP PUT request: +``` +curl --request PUT \ + --url http://localhost:8080/iox/api/v1/id \ + --header 'Content-Type: application/json' \ + --data '{ + "id": 42 + }' +``` + +To write data, you need a destination database. +This is set via HTTP PUT, identifying the database by org `company` and bucket `sensors`: +``` +curl --request PUT \ + --url http://localhost:8080/iox/api/v1/databases/company_sensors \ + --header 'Content-Type: application/json' \ + --data '{ + "store_locally": true +}' +``` + Data can be stored in InfluxDB IOx by sending it in [line protocol] format to the `/api/v2/write` endpoint. Data is stored by organization and bucket names. Here's an example using [`curl`] with the organization name `company` and the bucket name `sensors` that will send the data in the From 9323da43d96a353a6bed25250186ba10f8cc8f26 Mon Sep 17 00:00:00 2001 From: Jacob Marble Date: Thu, 25 Feb 2021 17:33:34 -0800 Subject: [PATCH 19/22] chore: fix example --- README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/README.md b/README.md index ca16f66e5c..a64ad54b5a 100644 --- a/README.md +++ b/README.md @@ -197,7 +197,6 @@ curl --request PUT \ --url http://localhost:8080/iox/api/v1/databases/company_sensors \ --header 'Content-Type: application/json' \ --data '{ - "store_locally": true }' ``` From fdcb8baec1cb83cd5d066fc4b3fbc53d17253456 Mon Sep 17 00:00:00 2001 From: Raphael Taylor-Davies <1781103+tustvold@users.noreply.github.com> Date: Fri, 26 Feb 2021 10:10:47 +0000 Subject: [PATCH 20/22] feat: conversion to/from data_types and generated_types (#848) --- data_types/src/database_rules.rs | 938 ++++++++++++++++++++++++++++- data_types/src/field_validation.rs | 112 ++++ data_types/src/lib.rs | 2 + 3 files changed, 1047 insertions(+), 5 deletions(-) create mode 100644 data_types/src/field_validation.rs diff --git a/data_types/src/database_rules.rs b/data_types/src/database_rules.rs index c11160d4e1..f66ba8ad93 100644 --- a/data_types/src/database_rules.rs +++ b/data_types/src/database_rules.rs @@ -1,9 +1,19 @@ -use influxdb_line_protocol::ParsedLine; +use std::convert::{TryFrom, TryInto}; use chrono::{DateTime, TimeZone, Utc}; use serde::{Deserialize, Serialize}; use snafu::Snafu; +use generated_types::google::protobuf::Empty; +use generated_types::{ + google::{FieldViolation, FieldViolationExt}, + influxdata::iox::management::v1 as management, +}; +use influxdb_line_protocol::ParsedLine; + +use crate::field_validation::{FromField, FromFieldOpt, FromFieldString, FromFieldVec}; +use crate::DatabaseName; + #[derive(Debug, Snafu)] pub enum Error { #[snafu(display("Error in {}: {}", source_module, source))] @@ -22,7 +32,7 @@ pub struct DatabaseRules { /// The unencoded name of the database. This gets put in by the create /// database call, so an empty default is fine. #[serde(default)] - pub name: String, + pub name: String, // TODO: Use DatabaseName here /// Template that generates a partition key for each row inserted into the /// db #[serde(default)] @@ -137,6 +147,82 @@ impl Partitioner for DatabaseRules { } } +impl From for management::DatabaseRules { + fn from(rules: DatabaseRules) -> Self { + let subscriptions: Vec = + rules.subscriptions.into_iter().map(Into::into).collect(); + + let replication_config = management::ReplicationConfig { + replications: rules.replication, + replication_count: rules.replication_count as _, + replication_queue_max_size: rules.replication_queue_max_size as _, + }; + + let query_config = management::QueryConfig { + query_local: rules.query_local, + primary: rules.primary_query_group.unwrap_or_default(), + secondaries: rules.secondary_query_groups, + read_only_partitions: rules.read_only_partitions, + }; + + Self { + name: rules.name, + partition_template: Some(rules.partition_template.into()), + replication_config: Some(replication_config), + subscription_config: Some(management::SubscriptionConfig { subscriptions }), + query_config: Some(query_config), + wal_buffer_config: rules.wal_buffer_config.map(Into::into), + mutable_buffer_config: rules.mutable_buffer_config.map(Into::into), + } + } +} + +impl TryFrom for DatabaseRules { + type Error = FieldViolation; + + fn try_from(proto: management::DatabaseRules) -> Result { + DatabaseName::new(&proto.name).field("name")?; + + let subscriptions = proto + .subscription_config + .map(|s| { + s.subscriptions + .vec_field("subscription_config.subscriptions") + }) + .transpose()? + .unwrap_or_default(); + + let wal_buffer_config = proto.wal_buffer_config.optional("wal_buffer_config")?; + + let mutable_buffer_config = proto + .mutable_buffer_config + .optional("mutable_buffer_config")?; + + let partition_template = proto + .partition_template + .optional("partition_template")? + .unwrap_or_default(); + + let query = proto.query_config.unwrap_or_default(); + let replication = proto.replication_config.unwrap_or_default(); + + Ok(Self { + name: proto.name, + partition_template, + replication: replication.replications, + replication_count: replication.replication_count as _, + replication_queue_max_size: replication.replication_queue_max_size as _, + subscriptions, + query_local: query.query_local, + primary_query_group: query.primary.optional(), + secondary_query_groups: query.secondaries, + read_only_partitions: query.read_only_partitions, + wal_buffer_config, + mutable_buffer_config, + }) + } +} + /// MutableBufferConfig defines the configuration for the in-memory database /// that is hot for writes as they arrive. Operators can define rules for /// evicting data once the mutable buffer passes a set memory threshold. @@ -190,6 +276,47 @@ impl Default for MutableBufferConfig { } } +impl From for management::MutableBufferConfig { + fn from(config: MutableBufferConfig) -> Self { + Self { + buffer_size: config.buffer_size as _, + reject_if_not_persisted: config.reject_if_not_persisted, + partition_drop_order: Some(config.partition_drop_order.into()), + persist_after_cold_seconds: config.persist_after_cold_seconds.unwrap_or_default(), + } + } +} + +impl TryFrom for MutableBufferConfig { + type Error = FieldViolation; + + fn try_from(proto: management::MutableBufferConfig) -> Result { + let partition_drop_order = proto + .partition_drop_order + .optional("partition_drop_order")? + .unwrap_or_default(); + + let buffer_size = if proto.buffer_size == 0 { + DEFAULT_MUTABLE_BUFFER_SIZE + } else { + proto.buffer_size as usize + }; + + let persist_after_cold_seconds = if proto.persist_after_cold_seconds == 0 { + None + } else { + Some(proto.persist_after_cold_seconds) + }; + + Ok(Self { + buffer_size, + reject_if_not_persisted: proto.reject_if_not_persisted, + partition_drop_order, + persist_after_cold_seconds, + }) + } +} + /// This struct specifies the rules for the order to sort partitions /// from the mutable buffer. This is used to determine which order to drop them /// in. The last partition in the list will be dropped, until enough space has @@ -204,7 +331,7 @@ impl Default for MutableBufferConfig { /// sort: PartitionSort::CreatedAtTime, /// }; /// ``` -#[derive(Debug, Serialize, Deserialize, Eq, PartialEq, Clone)] +#[derive(Debug, Default, Serialize, Deserialize, Eq, PartialEq, Clone)] pub struct PartitionSortRules { /// Sort partitions by this order. Last will be dropped. pub order: Order, @@ -213,6 +340,30 @@ pub struct PartitionSortRules { pub sort: PartitionSort, } +impl From for management::mutable_buffer_config::PartitionDropOrder { + fn from(ps: PartitionSortRules) -> Self { + let order: management::Order = ps.order.into(); + + Self { + order: order as _, + sort: Some(ps.sort.into()), + } + } +} + +impl TryFrom for PartitionSortRules { + type Error = FieldViolation; + + fn try_from( + proto: management::mutable_buffer_config::PartitionDropOrder, + ) -> Result { + Ok(Self { + order: proto.order().scope("order")?, + sort: proto.sort.optional("sort")?.unwrap_or_default(), + }) + } +} + /// What to sort the partition by. #[derive(Debug, Serialize, Deserialize, Eq, PartialEq, Clone)] pub enum PartitionSort { @@ -235,6 +386,57 @@ pub enum PartitionSort { Column(String, ColumnType, ColumnValue), } +impl Default for PartitionSort { + fn default() -> Self { + Self::LastWriteTime + } +} + +impl From for management::mutable_buffer_config::partition_drop_order::Sort { + fn from(ps: PartitionSort) -> Self { + use management::mutable_buffer_config::partition_drop_order::ColumnSort; + + match ps { + PartitionSort::LastWriteTime => Self::LastWriteTime(Empty {}), + PartitionSort::CreatedAtTime => Self::CreatedAtTime(Empty {}), + PartitionSort::Column(column_name, column_type, column_value) => { + let column_type: management::ColumnType = column_type.into(); + let column_value: management::Aggregate = column_value.into(); + + Self::Column(ColumnSort { + column_name, + column_type: column_type as _, + column_value: column_value as _, + }) + } + } + } +} + +impl TryFrom for PartitionSort { + type Error = FieldViolation; + + fn try_from( + proto: management::mutable_buffer_config::partition_drop_order::Sort, + ) -> Result { + use management::mutable_buffer_config::partition_drop_order::Sort; + + Ok(match proto { + Sort::LastWriteTime(_) => Self::LastWriteTime, + Sort::CreatedAtTime(_) => Self::CreatedAtTime, + Sort::Column(column_sort) => { + let column_type = column_sort.column_type().scope("column.column_type")?; + let column_value = column_sort.column_value().scope("column.column_value")?; + Self::Column( + column_sort.column_name.required("column.column_name")?, + column_type, + column_value, + ) + } + }) + } +} + /// The sort order. #[derive(Debug, Serialize, Deserialize, Eq, PartialEq, Clone)] pub enum Order { @@ -242,6 +444,33 @@ pub enum Order { Desc, } +impl Default for Order { + fn default() -> Self { + Self::Asc + } +} + +impl From for management::Order { + fn from(o: Order) -> Self { + match o { + Order::Asc => Self::Asc, + Order::Desc => Self::Desc, + } + } +} + +impl TryFrom for Order { + type Error = FieldViolation; + + fn try_from(proto: management::Order) -> Result { + Ok(match proto { + management::Order::Unspecified => Self::default(), + management::Order::Asc => Self::Asc, + management::Order::Desc => Self::Desc, + }) + } +} + /// Use columns of this type. #[derive(Debug, Serialize, Deserialize, Eq, PartialEq, Clone)] pub enum ColumnType { @@ -252,6 +481,33 @@ pub enum ColumnType { Bool, } +impl From for management::ColumnType { + fn from(t: ColumnType) -> Self { + match t { + ColumnType::I64 => Self::I64, + ColumnType::U64 => Self::U64, + ColumnType::F64 => Self::F64, + ColumnType::String => Self::String, + ColumnType::Bool => Self::Bool, + } + } +} + +impl TryFrom for ColumnType { + type Error = FieldViolation; + + fn try_from(proto: management::ColumnType) -> Result { + Ok(match proto { + management::ColumnType::Unspecified => return Err(FieldViolation::required("")), + management::ColumnType::I64 => Self::I64, + management::ColumnType::U64 => Self::U64, + management::ColumnType::F64 => Self::F64, + management::ColumnType::String => Self::String, + management::ColumnType::Bool => Self::Bool, + }) + } +} + /// Use either the min or max summary statistic. #[derive(Debug, Serialize, Deserialize, Eq, PartialEq, Clone)] pub enum ColumnValue { @@ -259,6 +515,29 @@ pub enum ColumnValue { Max, } +impl From for management::Aggregate { + fn from(v: ColumnValue) -> Self { + match v { + ColumnValue::Min => Self::Min, + ColumnValue::Max => Self::Max, + } + } +} + +impl TryFrom for ColumnValue { + type Error = FieldViolation; + + fn try_from(proto: management::Aggregate) -> Result { + use management::Aggregate; + + Ok(match proto { + Aggregate::Unspecified => return Err(FieldViolation::required("")), + Aggregate::Min => Self::Min, + Aggregate::Max => Self::Max, + }) + } +} + /// WalBufferConfig defines the configuration for buffering data from the WAL in /// memory. This buffer is used for asynchronous replication and to collect /// segments before sending them to object storage. @@ -294,6 +573,45 @@ pub struct WalBufferConfig { pub close_segment_after: Option, } +impl From for management::WalBufferConfig { + fn from(rollover: WalBufferConfig) -> Self { + let buffer_rollover: management::wal_buffer_config::Rollover = + rollover.buffer_rollover.into(); + + Self { + buffer_size: rollover.buffer_size, + segment_size: rollover.segment_size, + buffer_rollover: buffer_rollover as _, + persist_segments: rollover.store_segments, + close_segment_after: rollover.close_segment_after.map(Into::into), + } + } +} + +impl TryFrom for WalBufferConfig { + type Error = FieldViolation; + + fn try_from(proto: management::WalBufferConfig) -> Result { + let buffer_rollover = proto.buffer_rollover().scope("buffer_rollover")?; + let close_segment_after = proto + .close_segment_after + .map(TryInto::try_into) + .transpose() + .map_err(|_| FieldViolation { + field: "closeSegmentAfter".to_string(), + description: "Duration must be positive".to_string(), + })?; + + Ok(Self { + buffer_size: proto.buffer_size, + segment_size: proto.segment_size, + buffer_rollover, + store_segments: proto.persist_segments, + close_segment_after, + }) + } +} + /// WalBufferRollover defines the behavior of what should happen if a write /// comes in that would cause the buffer to exceed its max size AND the oldest /// segment can't be dropped because it has not yet been persisted. @@ -311,6 +629,30 @@ pub enum WalBufferRollover { ReturnError, } +impl From for management::wal_buffer_config::Rollover { + fn from(rollover: WalBufferRollover) -> Self { + match rollover { + WalBufferRollover::DropOldSegment => Self::DropOldSegment, + WalBufferRollover::DropIncoming => Self::DropIncoming, + WalBufferRollover::ReturnError => Self::ReturnError, + } + } +} + +impl TryFrom for WalBufferRollover { + type Error = FieldViolation; + + fn try_from(proto: management::wal_buffer_config::Rollover) -> Result { + use management::wal_buffer_config::Rollover; + Ok(match proto { + Rollover::Unspecified => return Err(FieldViolation::required("")), + Rollover::DropOldSegment => Self::DropOldSegment, + Rollover::DropIncoming => Self::DropIncoming, + Rollover::ReturnError => Self::ReturnError, + }) + } +} + /// `PartitionTemplate` is used to compute the partition key of each row that /// gets written. It can consist of the table name, a column name and its value, /// a formatted time, or a string column and regex captures of its value. For @@ -353,6 +695,23 @@ impl PartitionTemplate { } } +impl From for management::PartitionTemplate { + fn from(pt: PartitionTemplate) -> Self { + Self { + parts: pt.parts.into_iter().map(Into::into).collect(), + } + } +} + +impl TryFrom for PartitionTemplate { + type Error = FieldViolation; + + fn try_from(proto: management::PartitionTemplate) -> Result { + let parts = proto.parts.vec_field("parts")?; + Ok(Self { parts }) + } +} + /// `TemplatePart` specifies what part of a row should be used to compute this /// part of a partition key. #[derive(Debug, Serialize, Deserialize, Eq, PartialEq, Clone)] @@ -380,6 +739,67 @@ pub struct StrftimeColumn { format: String, } +impl From for management::partition_template::part::Part { + fn from(part: TemplatePart) -> Self { + use management::partition_template::part::ColumnFormat; + + match part { + TemplatePart::Table => Self::Table(Empty {}), + TemplatePart::Column(column) => Self::Column(column), + TemplatePart::RegexCapture(RegexCapture { column, regex }) => { + Self::Regex(ColumnFormat { + column, + format: regex, + }) + } + TemplatePart::StrftimeColumn(StrftimeColumn { column, format }) => { + Self::StrfTime(ColumnFormat { column, format }) + } + TemplatePart::TimeFormat(format) => Self::Time(format), + } + } +} + +impl TryFrom for TemplatePart { + type Error = FieldViolation; + + fn try_from(proto: management::partition_template::part::Part) -> Result { + use management::partition_template::part::{ColumnFormat, Part}; + + Ok(match proto { + Part::Table(_) => Self::Table, + Part::Column(column) => Self::Column(column.required("column")?), + Part::Regex(ColumnFormat { column, format }) => Self::RegexCapture(RegexCapture { + column: column.required("regex.column")?, + regex: format.required("regex.format")?, + }), + Part::StrfTime(ColumnFormat { column, format }) => { + Self::StrftimeColumn(StrftimeColumn { + column: column.required("strf_time.column")?, + format: format.required("strf_time.format")?, + }) + } + Part::Time(format) => Self::TimeFormat(format.required("time")?), + }) + } +} + +impl From for management::partition_template::Part { + fn from(part: TemplatePart) -> Self { + Self { + part: Some(part.into()), + } + } +} + +impl TryFrom for TemplatePart { + type Error = FieldViolation; + + fn try_from(proto: management::partition_template::Part) -> Result { + proto.part.required("part") + } +} + /// `PartitionId` is the object storage identifier for a specific partition. It /// should be a path that can be used against an object store to locate all the /// files and subdirectories for a partition. It takes the form of `/ for management::subscription_config::Subscription { + fn from(s: Subscription) -> Self { + Self { + name: s.name, + host_group_id: s.host_group_id, + matcher: Some(s.matcher.into()), + } + } +} + +impl TryFrom for Subscription { + type Error = FieldViolation; + + fn try_from(proto: management::subscription_config::Subscription) -> Result { + Ok(Self { + name: proto.name.required("name")?, + host_group_id: proto.host_group_id.required("host_group_id")?, + matcher: proto.matcher.optional("matcher")?.unwrap_or_default(), + }) + } +} + /// `Matcher` specifies the rule against the table name and/or a predicate /// against the row to determine if it matches the write rule. -#[derive(Debug, Serialize, Deserialize, Eq, PartialEq, Clone)] +#[derive(Debug, Default, Serialize, Deserialize, Eq, PartialEq, Clone)] pub struct Matcher { pub tables: MatchTables, // TODO: make this work with query::Predicate @@ -413,6 +855,26 @@ pub struct Matcher { pub predicate: Option, } +impl From for management::Matcher { + fn from(m: Matcher) -> Self { + Self { + predicate: m.predicate.unwrap_or_default(), + table_matcher: Some(m.tables.into()), + } + } +} + +impl TryFrom for Matcher { + type Error = FieldViolation; + + fn try_from(proto: management::Matcher) -> Result { + Ok(Self { + tables: proto.table_matcher.required("table_matcher")?, + predicate: proto.predicate.optional(), + }) + } +} + /// `MatchTables` looks at the table name of a row to determine if it should /// match the rule. #[derive(Debug, Serialize, Deserialize, Eq, PartialEq, Clone)] @@ -424,6 +886,35 @@ pub enum MatchTables { Regex(String), } +impl Default for MatchTables { + fn default() -> Self { + Self::All + } +} + +impl From for management::matcher::TableMatcher { + fn from(m: MatchTables) -> Self { + match m { + MatchTables::All => Self::All(Empty {}), + MatchTables::Table(table) => Self::Table(table), + MatchTables::Regex(regex) => Self::Regex(regex), + } + } +} + +impl TryFrom for MatchTables { + type Error = FieldViolation; + + fn try_from(proto: management::matcher::TableMatcher) -> Result { + use management::matcher::TableMatcher; + Ok(match proto { + TableMatcher::All(_) => Self::All, + TableMatcher::Table(table) => Self::Table(table.required("table_matcher.table")?), + TableMatcher::Regex(regex) => Self::Regex(regex.required("table_matcher.regex")?), + }) + } +} + pub type HostGroupId = String; #[derive(Debug, Serialize, Deserialize, Eq, PartialEq, Clone)] @@ -435,9 +926,10 @@ pub struct HostGroup { #[cfg(test)] mod tests { - use super::*; use influxdb_line_protocol::parse_lines; + use super::*; + type TestError = Box; type Result = std::result::Result; @@ -598,4 +1090,440 @@ mod tests { fn parse_line(line: &str) -> ParsedLine<'_> { parsed_lines(line).pop().unwrap() } + + #[test] + fn test_database_rules_defaults() { + let protobuf = management::DatabaseRules { + name: "database".to_string(), + ..Default::default() + }; + + let rules: DatabaseRules = protobuf.clone().try_into().unwrap(); + let back: management::DatabaseRules = rules.clone().into(); + + assert_eq!(rules.name, protobuf.name); + assert_eq!(protobuf.name, back.name); + + assert_eq!(rules.partition_template.parts.len(), 0); + assert_eq!(rules.subscriptions.len(), 0); + assert!(rules.primary_query_group.is_none()); + assert_eq!(rules.read_only_partitions.len(), 0); + assert_eq!(rules.secondary_query_groups.len(), 0); + + // These will be defaulted as optionality not preserved on non-protobuf + // DatabaseRules + assert_eq!(back.replication_config, Some(Default::default())); + assert_eq!(back.subscription_config, Some(Default::default())); + assert_eq!(back.query_config, Some(Default::default())); + assert_eq!(back.partition_template, Some(Default::default())); + + // These should be none as preserved on non-protobuf DatabaseRules + assert!(back.wal_buffer_config.is_none()); + assert!(back.mutable_buffer_config.is_none()); + } + + #[test] + fn test_database_rules_query() { + let readonly = vec!["readonly1".to_string(), "readonly2".to_string()]; + let secondaries = vec!["secondary1".to_string(), "secondary2".to_string()]; + + let protobuf = management::DatabaseRules { + name: "database".to_string(), + query_config: Some(management::QueryConfig { + query_local: true, + primary: "primary".to_string(), + secondaries: secondaries.clone(), + read_only_partitions: readonly.clone(), + }), + ..Default::default() + }; + + let rules: DatabaseRules = protobuf.clone().try_into().unwrap(); + let back: management::DatabaseRules = rules.clone().into(); + + assert_eq!(rules.name, protobuf.name); + assert_eq!(protobuf.name, back.name); + + assert_eq!(rules.read_only_partitions, readonly); + assert_eq!(rules.primary_query_group, Some("primary".to_string())); + assert_eq!(rules.secondary_query_groups, secondaries); + assert_eq!(rules.subscriptions.len(), 0); + assert_eq!(rules.partition_template.parts.len(), 0); + + // Should be the same as was specified + assert_eq!(back.query_config, protobuf.query_config); + assert!(back.wal_buffer_config.is_none()); + assert!(back.mutable_buffer_config.is_none()); + + // These will be defaulted as optionality not preserved on non-protobuf + // DatabaseRules + assert_eq!(back.replication_config, Some(Default::default())); + assert_eq!(back.subscription_config, Some(Default::default())); + assert_eq!(back.partition_template, Some(Default::default())); + } + + #[test] + fn test_query_config_default() { + let protobuf = management::DatabaseRules { + name: "database".to_string(), + query_config: Some(Default::default()), + ..Default::default() + }; + + let rules: DatabaseRules = protobuf.clone().try_into().unwrap(); + let back: management::DatabaseRules = rules.clone().into(); + + assert!(rules.primary_query_group.is_none()); + assert_eq!(rules.secondary_query_groups.len(), 0); + assert_eq!(rules.read_only_partitions.len(), 0); + assert_eq!(rules.query_local, false); + + assert_eq!(protobuf.query_config, back.query_config); + } + + #[test] + fn test_partition_template_default() { + let protobuf = management::DatabaseRules { + name: "database".to_string(), + partition_template: Some(management::PartitionTemplate { parts: vec![] }), + ..Default::default() + }; + + let rules: DatabaseRules = protobuf.clone().try_into().unwrap(); + let back: management::DatabaseRules = rules.clone().into(); + + assert_eq!(rules.partition_template.parts.len(), 0); + assert_eq!(protobuf.partition_template, back.partition_template); + } + + #[test] + fn test_partition_template_no_part() { + let protobuf = management::DatabaseRules { + name: "database".to_string(), + partition_template: Some(management::PartitionTemplate { + parts: vec![Default::default()], + }), + ..Default::default() + }; + + let res: Result = protobuf.try_into(); + let err = res.expect_err("expected failure"); + + assert_eq!(&err.field, "partition_template.parts.0.part"); + assert_eq!(&err.description, "Field is required"); + } + + #[test] + fn test_partition_template() { + use management::partition_template::part::{ColumnFormat, Part}; + + let protobuf = management::PartitionTemplate { + parts: vec![ + management::partition_template::Part { + part: Some(Part::Time("time".to_string())), + }, + management::partition_template::Part { + part: Some(Part::Table(Empty {})), + }, + management::partition_template::Part { + part: Some(Part::Regex(ColumnFormat { + column: "column".to_string(), + format: "format".to_string(), + })), + }, + ], + }; + + let pt: PartitionTemplate = protobuf.clone().try_into().unwrap(); + let back: management::PartitionTemplate = pt.clone().into(); + + assert_eq!( + pt.parts, + vec![ + TemplatePart::TimeFormat("time".to_string()), + TemplatePart::Table, + TemplatePart::RegexCapture(RegexCapture { + column: "column".to_string(), + regex: "format".to_string() + }) + ] + ); + assert_eq!(protobuf, back); + } + + #[test] + fn test_partition_template_empty() { + use management::partition_template::part::{ColumnFormat, Part}; + + let protobuf = management::PartitionTemplate { + parts: vec![management::partition_template::Part { + part: Some(Part::Regex(ColumnFormat { + ..Default::default() + })), + }], + }; + + let res: Result = protobuf.try_into(); + let err = res.expect_err("expected failure"); + + assert_eq!(&err.field, "parts.0.part.regex.column"); + assert_eq!(&err.description, "Field is required"); + } + + #[test] + fn test_wal_buffer_config_default() { + let protobuf: management::WalBufferConfig = Default::default(); + + let res: Result = protobuf.try_into(); + let err = res.expect_err("expected failure"); + + assert_eq!(&err.field, "buffer_rollover"); + assert_eq!(&err.description, "Field is required"); + } + + #[test] + fn test_wal_buffer_config_rollover() { + let protobuf = management::WalBufferConfig { + buffer_rollover: management::wal_buffer_config::Rollover::DropIncoming as _, + ..Default::default() + }; + + let config: WalBufferConfig = protobuf.clone().try_into().unwrap(); + let back: management::WalBufferConfig = config.clone().into(); + + assert_eq!(config.buffer_rollover, WalBufferRollover::DropIncoming); + assert_eq!(protobuf, back); + } + + #[test] + fn test_wal_buffer_config_negative_duration() { + use generated_types::google::protobuf::Duration; + + let protobuf = management::WalBufferConfig { + buffer_rollover: management::wal_buffer_config::Rollover::DropOldSegment as _, + close_segment_after: Some(Duration { + seconds: -1, + nanos: -40, + }), + ..Default::default() + }; + + let res: Result = protobuf.try_into(); + let err = res.expect_err("expected failure"); + + assert_eq!(&err.field, "closeSegmentAfter"); + assert_eq!(&err.description, "Duration must be positive"); + } + + #[test] + fn test_matcher_default() { + let protobuf: management::Matcher = Default::default(); + + let res: Result = protobuf.try_into(); + let err = res.expect_err("expected failure"); + + assert_eq!(&err.field, "table_matcher"); + assert_eq!(&err.description, "Field is required"); + } + + #[test] + fn test_matcher() { + let protobuf = management::Matcher { + predicate: Default::default(), + table_matcher: Some(management::matcher::TableMatcher::Regex( + "regex".to_string(), + )), + }; + let matcher: Matcher = protobuf.try_into().unwrap(); + + assert_eq!(matcher.tables, MatchTables::Regex("regex".to_string())); + assert!(matcher.predicate.is_none()); + } + + #[test] + fn test_subscription_default() { + let pb_matcher = Some(management::Matcher { + predicate: "predicate1".to_string(), + table_matcher: Some(management::matcher::TableMatcher::Table( + "table".to_string(), + )), + }); + + let matcher = Matcher { + tables: MatchTables::Table("table".to_string()), + predicate: Some("predicate1".to_string()), + }; + + let subscription_config = management::SubscriptionConfig { + subscriptions: vec![ + management::subscription_config::Subscription { + name: "subscription1".to_string(), + host_group_id: "host group".to_string(), + matcher: pb_matcher.clone(), + }, + management::subscription_config::Subscription { + name: "subscription2".to_string(), + host_group_id: "host group".to_string(), + matcher: pb_matcher, + }, + ], + }; + + let protobuf = management::DatabaseRules { + name: "database".to_string(), + subscription_config: Some(subscription_config), + ..Default::default() + }; + + let rules: DatabaseRules = protobuf.clone().try_into().unwrap(); + let back: management::DatabaseRules = rules.clone().into(); + + assert_eq!(protobuf.subscription_config, back.subscription_config); + assert_eq!( + rules.subscriptions, + vec![ + Subscription { + name: "subscription1".to_string(), + host_group_id: "host group".to_string(), + matcher: matcher.clone() + }, + Subscription { + name: "subscription2".to_string(), + host_group_id: "host group".to_string(), + matcher + } + ] + ) + } + + #[test] + fn mutable_buffer_config_default() { + let protobuf: management::MutableBufferConfig = Default::default(); + + let config: MutableBufferConfig = protobuf.try_into().unwrap(); + let back: management::MutableBufferConfig = config.clone().into(); + + assert_eq!(config.buffer_size, DEFAULT_MUTABLE_BUFFER_SIZE); + assert_eq!(config.persist_after_cold_seconds, None); + assert_eq!(config.partition_drop_order, PartitionSortRules::default()); + assert!(!config.reject_if_not_persisted); + + assert_eq!(back.reject_if_not_persisted, config.reject_if_not_persisted); + assert_eq!(back.buffer_size as usize, config.buffer_size); + assert_eq!( + back.partition_drop_order, + Some(PartitionSortRules::default().into()) + ); + assert_eq!(back.persist_after_cold_seconds, 0); + } + + #[test] + fn mutable_buffer_config() { + let protobuf = management::MutableBufferConfig { + buffer_size: 32, + reject_if_not_persisted: true, + partition_drop_order: Some(management::mutable_buffer_config::PartitionDropOrder { + order: management::Order::Desc as _, + sort: None, + }), + persist_after_cold_seconds: 439, + }; + + let config: MutableBufferConfig = protobuf.clone().try_into().unwrap(); + let back: management::MutableBufferConfig = config.clone().into(); + + assert_eq!(config.buffer_size, protobuf.buffer_size as usize); + assert_eq!( + config.persist_after_cold_seconds, + Some(protobuf.persist_after_cold_seconds) + ); + assert_eq!(config.partition_drop_order.order, Order::Desc); + assert!(config.reject_if_not_persisted); + + assert_eq!(back.reject_if_not_persisted, config.reject_if_not_persisted); + assert_eq!(back.buffer_size as usize, config.buffer_size); + assert_eq!( + back.persist_after_cold_seconds, + protobuf.persist_after_cold_seconds + ); + } + + #[test] + fn partition_drop_order_default() { + let protobuf: management::mutable_buffer_config::PartitionDropOrder = Default::default(); + let config: PartitionSortRules = protobuf.try_into().unwrap(); + + assert_eq!(config, PartitionSortRules::default()); + assert_eq!(config.order, Order::default()); + assert_eq!(config.sort, PartitionSort::default()); + } + + #[test] + fn partition_drop_order() { + use management::mutable_buffer_config::{partition_drop_order::Sort, PartitionDropOrder}; + let protobuf = PartitionDropOrder { + order: management::Order::Asc as _, + sort: Some(Sort::CreatedAtTime(Empty {})), + }; + let config: PartitionSortRules = protobuf.clone().try_into().unwrap(); + let back: PartitionDropOrder = config.clone().into(); + + assert_eq!(protobuf, back); + assert_eq!(config.order, Order::Asc); + assert_eq!(config.sort, PartitionSort::CreatedAtTime); + } + + #[test] + fn partition_sort() { + use management::mutable_buffer_config::partition_drop_order::{ColumnSort, Sort}; + + let created_at: PartitionSort = Sort::CreatedAtTime(Empty {}).try_into().unwrap(); + let last_write: PartitionSort = Sort::LastWriteTime(Empty {}).try_into().unwrap(); + let column: PartitionSort = Sort::Column(ColumnSort { + column_name: "column".to_string(), + column_type: management::ColumnType::Bool as _, + column_value: management::Aggregate::Min as _, + }) + .try_into() + .unwrap(); + + assert_eq!(created_at, PartitionSort::CreatedAtTime); + assert_eq!(last_write, PartitionSort::LastWriteTime); + assert_eq!( + column, + PartitionSort::Column("column".to_string(), ColumnType::Bool, ColumnValue::Min) + ); + } + + #[test] + fn partition_sort_column_sort() { + use management::mutable_buffer_config::partition_drop_order::{ColumnSort, Sort}; + + let res: Result = Sort::Column(Default::default()).try_into(); + let err1 = res.expect_err("expected failure"); + + let res: Result = Sort::Column(ColumnSort { + column_type: management::ColumnType::F64 as _, + ..Default::default() + }) + .try_into(); + let err2 = res.expect_err("expected failure"); + + let res: Result = Sort::Column(ColumnSort { + column_type: management::ColumnType::F64 as _, + column_value: management::Aggregate::Max as _, + ..Default::default() + }) + .try_into(); + let err3 = res.expect_err("expected failure"); + + assert_eq!(err1.field, "column.column_type"); + assert_eq!(err1.description, "Field is required"); + + assert_eq!(err2.field, "column.column_value"); + assert_eq!(err2.description, "Field is required"); + + assert_eq!(err3.field, "column.column_name"); + assert_eq!(err3.description, "Field is required"); + } } diff --git a/data_types/src/field_validation.rs b/data_types/src/field_validation.rs new file mode 100644 index 0000000000..49ae635a6d --- /dev/null +++ b/data_types/src/field_validation.rs @@ -0,0 +1,112 @@ +//! A collection of extension traits for types that +//! implement TryInto +//! +//! Allows associating field context with the generated errors +//! as they propagate up the struct topology + +use generated_types::google::FieldViolation; +use std::convert::TryInto; + +/// An extension trait that adds the method `scope` to any type +/// implementing `TryInto` +pub(crate) trait FromField { + fn scope(self, field: impl Into) -> Result; +} + +impl FromField for T +where + T: TryInto, +{ + /// Try to convert type using TryInto calling `FieldViolation::scope` + /// on any returned error + fn scope(self, field: impl Into) -> Result { + self.try_into().map_err(|e| e.scope(field)) + } +} + +/// An extension trait that adds the methods `optional` and `required` to any +/// Option containing a type implementing `TryInto` +pub(crate) trait FromFieldOpt { + /// Try to convert inner type, if any, using TryInto calling + /// `FieldViolation::scope` on any error encountered + /// + /// Returns None if empty + fn optional(self, field: impl Into) -> Result, FieldViolation>; + + /// Try to convert inner type, using TryInto calling `FieldViolation::scope` + /// on any error encountered + /// + /// Returns an error if empty + fn required(self, field: impl Into) -> Result; +} + +impl FromFieldOpt for Option +where + T: TryInto, +{ + fn optional(self, field: impl Into) -> Result, FieldViolation> { + self.map(|t| t.scope(field)).transpose() + } + + fn required(self, field: impl Into) -> Result { + match self { + None => Err(FieldViolation::required(field)), + Some(t) => t.scope(field), + } + } +} + +/// An extension trait that adds the methods `optional` and `required` to any +/// String +/// +/// Prost will default string fields to empty, whereas IOx sometimes +/// uses Option, this helper aids mapping between them +/// +/// TODO: Review mixed use of Option and String in IOX +pub(crate) trait FromFieldString { + /// Returns a Ok if the String is not empty + fn required(self, field: impl Into) -> Result; + + /// Wraps non-empty strings in Some(_), returns None for empty strings + fn optional(self) -> Option; +} + +impl FromFieldString for String { + fn required(self, field: impl Into) -> Result { + if self.is_empty() { + return Err(FieldViolation::required(field)); + } + Ok(self) + } + + fn optional(self) -> Option { + if self.is_empty() { + return None; + } + Some(self) + } +} + +/// An extension trait that adds the method `vec_field` to any Vec of a type +/// implementing `TryInto` +pub(crate) trait FromFieldVec { + /// Converts to a `Vec`, short-circuiting on the first error and + /// returning a correctly scoped `FieldViolation` for where the error + /// was encountered + fn vec_field(self, field: impl Into) -> Result; +} + +impl FromFieldVec> for Vec +where + T: TryInto, +{ + fn vec_field(self, field: impl Into) -> Result, FieldViolation> { + let res: Result<_, _> = self + .into_iter() + .enumerate() + .map(|(i, t)| t.scope(i.to_string())) + .collect(); + + res.map_err(|e| e.scope(field)) + } +} diff --git a/data_types/src/lib.rs b/data_types/src/lib.rs index 7cf358c180..d3766053b6 100644 --- a/data_types/src/lib.rs +++ b/data_types/src/lib.rs @@ -32,3 +32,5 @@ pub mod wal; mod database_name; pub use database_name::*; + +pub(crate) mod field_validation; From eca92b69daa2d89bb9db2607a0df040498984fd8 Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Fri, 26 Feb 2021 06:43:03 -0500 Subject: [PATCH 21/22] feat: implement format parameter on sql query endpoint (#870) * feat: implement format parameter on sql query endpoint * feat: include Content-Type header * fix: return proper JSON * test: add tests for JsonArrayWriter * refactor: remove use of tempfile * fix: clippy / fmt --- src/influxdb_ioxd/http.rs | 286 +++++++++++++++++++++-------- src/influxdb_ioxd/http/format.rs | 242 ++++++++++++++++++++++++ tests/end-to-end.rs | 2 +- tests/end_to_end_cases/read_api.rs | 15 +- 4 files changed, 461 insertions(+), 84 deletions(-) create mode 100644 src/influxdb_ioxd/http/format.rs diff --git a/src/influxdb_ioxd/http.rs b/src/influxdb_ioxd/http.rs index 8402fc2b1b..8c2bebe77d 100644 --- a/src/influxdb_ioxd/http.rs +++ b/src/influxdb_ioxd/http.rs @@ -1,5 +1,6 @@ -//! This module contains a partial implementation of the /v2 HTTP api -//! routes for InfluxDB IOx. +//! This module contains the HTTP api for InfluxDB IOx, including a +//! partial implementation of the /v2 HTTP api routes from InfluxDB +//! for compatibility. //! //! Note that these routes are designed to be just helpers for now, //! and "close enough" to the real /v2 api to be able to test InfluxDB IOx @@ -10,7 +11,7 @@ //! database names and may remove this quasi /v2 API. // Influx crates -use arrow_deps::{arrow, datafusion::physical_plan::collect}; +use arrow_deps::datafusion::physical_plan::collect; use data_types::{ database_rules::DatabaseRules, http::{ListDatabasesResponse, WalMetadataQuery}, @@ -25,7 +26,7 @@ use server::{ConnectionManager, Server as AppServer}; // External crates use bytes::{Bytes, BytesMut}; use futures::{self, StreamExt}; -use http::header::CONTENT_ENCODING; +use http::header::{CONTENT_ENCODING, CONTENT_TYPE}; use hyper::{Body, Method, Request, Response, StatusCode}; use routerify::{prelude::*, Middleware, RequestInfo, Router, RouterError, RouterService}; use serde::{Deserialize, Serialize}; @@ -35,6 +36,9 @@ use tracing::{debug, error, info}; use data_types::http::WalMetadataResponse; use std::{fmt::Debug, str, sync::Arc}; +mod format; +use format::QueryOutputFormat; + #[derive(Debug, Snafu)] pub enum ApplicationError { // Internal (unexpected) errors @@ -86,7 +90,9 @@ pub enum ApplicationError { #[snafu(display("Expected query string in request, but none was provided"))] ExpectedQueryString {}, - #[snafu(display("Invalid query string '{}': {}", query_string, source))] + /// Error for when we could not parse the http query uri (e.g. + /// `?foo=bar&bar=baz)` + #[snafu(display("Invalid query string in HTTP URI '{}': {}", query_string, source))] InvalidQueryString { query_string: String, source: serde_urlencoded::de::Error, @@ -151,6 +157,21 @@ pub enum ApplicationError { #[snafu(display("Database {} does not have a WAL", name))] WALNotFound { name: String }, + + #[snafu(display("Internal error creating HTTP response: {}", source))] + CreatingResponse { source: http::Error }, + + #[snafu(display( + "Error formatting results of SQL query '{}' using '{:?}': {}", + q, + format, + source + ))] + FormattingResult { + q: String, + format: QueryOutputFormat, + source: format::Error, + }, } impl ApplicationError { @@ -181,6 +202,8 @@ impl ApplicationError { Self::DatabaseNameError { .. } => self.bad_request(), Self::DatabaseNotFound { .. } => self.not_found(), Self::WALNotFound { .. } => self.not_found(), + Self::CreatingResponse { .. } => self.internal_error(), + Self::FormattingResult { .. } => self.internal_error(), } } @@ -260,7 +283,6 @@ where .post("/api/v2/write", write::) .get("/ping", ping) .get("/health", health) - .get("/api/v2/read", read::) .get("/iox/api/v1/databases", list_databases::) .put("/iox/api/v1/databases/:name", create_database::) .get("/iox/api/v1/databases/:name", get_database::) @@ -408,80 +430,35 @@ where .unwrap()) } -#[derive(Deserialize, Debug)] -/// Body of the request to the /read endpoint -struct ReadInfo { - org: String, - bucket: String, - // TODO This is currently a "SQL" request -- should be updated to conform - // to the V2 API for reading (using timestamps, etc). - sql_query: String, -} - -// TODO: figure out how to stream read results out rather than rendering the -// whole thing in mem -#[tracing::instrument(level = "debug")] -async fn read( - req: Request, -) -> Result, ApplicationError> { - let server = Arc::clone(&req.data::>>().expect("server state")); - let query = req.uri().query().context(ExpectedQueryString {})?; - - let read_info: ReadInfo = serde_urlencoded::from_str(query).context(InvalidQueryString { - query_string: query, - })?; - - let planner = SQLQueryPlanner::default(); - let executor = server.executor(); - - let db_name = org_and_bucket_to_database(&read_info.org, &read_info.bucket) - .context(BucketMappingError)?; - - let db = server.db(&db_name).await.context(BucketNotFound { - org: read_info.org.clone(), - bucket: read_info.bucket.clone(), - })?; - - let physical_plan = planner - .query(db.as_ref(), &read_info.sql_query, executor.as_ref()) - .await - .context(PlanningSQLQuery { query })?; - - let batches = collect(physical_plan) - .await - .map_err(|e| Box::new(e) as _) - .context(Query { db_name })?; - - let results = arrow::util::pretty::pretty_format_batches(&batches).unwrap(); - - Ok(Response::new(Body::from(results.into_bytes()))) -} - -#[derive(Deserialize, Debug)] -/// Body of the request to the .../query endpoint -struct QueryInfo { +#[derive(Deserialize, Debug, PartialEq)] +/// Parsed URI Parameters of the request to the .../query endpoint +struct QueryParams { q: String, + #[serde(default)] + format: QueryOutputFormat, } -// TODO: figure out how to stream read results out rather than rendering the -// whole thing in mem #[tracing::instrument(level = "debug")] async fn query( req: Request, ) -> Result, ApplicationError> { let server = Arc::clone(&req.data::>>().expect("server state")); - let query = req.uri().query().context(ExpectedQueryString {})?; + let uri_query = req.uri().query().context(ExpectedQueryString {})?; - let query_info: QueryInfo = serde_urlencoded::from_str(query).context(InvalidQueryString { - query_string: query, - })?; + let QueryParams { q, format } = + serde_urlencoded::from_str(uri_query).context(InvalidQueryString { + query_string: uri_query, + })?; let db_name_str = req .param("name") - .expect("db name must have been set") + .expect("db name must have been set by routerify") .clone(); + let db_name = DatabaseName::new(&db_name_str).context(DatabaseNameError)?; + debug!(uri = ?req.uri(), %q, ?format, %db_name, "running SQL query"); + let db = server .db(&db_name) .await @@ -491,18 +468,29 @@ async fn query( let executor = server.executor(); let physical_plan = planner - .query(db.as_ref(), &query_info.q, executor.as_ref()) + .query(db.as_ref(), &q, executor.as_ref()) .await - .context(PlanningSQLQuery { query })?; + .context(PlanningSQLQuery { query: &q })?; + // TODO: stream read results out rather than rendering the + // whole thing in mem let batches = collect(physical_plan) .await .map_err(|e| Box::new(e) as _) .context(Query { db_name })?; - let results = arrow::util::pretty::pretty_format_batches(&batches).unwrap(); + let results = format + .format(&batches) + .context(FormattingResult { q, format })?; - Ok(Response::new(Body::from(results.into_bytes()))) + let body = Body::from(results.into_bytes()); + + let response = Response::builder() + .header(CONTENT_TYPE, format.content_type()) + .body(body) + .context(CreatingResponse)?; + + Ok(response) } #[tracing::instrument(level = "debug")] @@ -805,7 +793,6 @@ mod tests { use std::net::{IpAddr, Ipv4Addr, SocketAddr}; use arrow_deps::{arrow::record_batch::RecordBatch, assert_table_eq}; - use http::header; use query::exec::Executor; use reqwest::{Client, Response}; @@ -905,9 +892,11 @@ mod tests { Ok(()) } - #[tokio::test] - async fn test_query() -> Result<()> { - let test_storage = Arc::new(AppServer::new( + /// Sets up a test database with some data for testing the query endpoint + /// returns a client for communicting with the server, and the server + /// endpoint + async fn setup_test_data() -> (Client, String) { + let test_storage: Arc> = Arc::new(AppServer::new( ConnectionManagerImpl {}, Arc::new(ObjectStore::new_in_memory(InMemory::new())), )); @@ -935,6 +924,12 @@ mod tests { .await; check_response("write", response, StatusCode::NO_CONTENT, "").await; + (client, server_url) + } + + #[tokio::test] + async fn test_query_pretty() -> Result<()> { + let (client, server_url) = setup_test_data().await; // send query data let response = client @@ -945,6 +940,8 @@ mod tests { .send() .await; + assert_eq!(get_content_type(&response), "text/plain"); + let res = "+----------------+--------------+-------+-----------------+------------+\n\ | bottom_degrees | location | state | surface_degrees | time |\n\ +----------------+--------------+-------+-----------------+------------+\n\ @@ -952,6 +949,79 @@ mod tests { +----------------+--------------+-------+-----------------+------------+\n"; check_response("query", response, StatusCode::OK, res).await; + // same response is expected if we explicitly request 'format=pretty' + let response = client + .get(&format!( + "{}/iox/api/v1/databases/MyOrg_MyBucket/query?q={}&format=pretty", + server_url, "select%20*%20from%20h2o_temperature" + )) + .send() + .await; + assert_eq!(get_content_type(&response), "text/plain"); + + check_response("query", response, StatusCode::OK, res).await; + + Ok(()) + } + + #[tokio::test] + async fn test_query_csv() -> Result<()> { + let (client, server_url) = setup_test_data().await; + + // send query data + let response = client + .get(&format!( + "{}/iox/api/v1/databases/MyOrg_MyBucket/query?q={}&format=csv", + server_url, "select%20*%20from%20h2o_temperature" + )) + .send() + .await; + + assert_eq!(get_content_type(&response), "text/csv"); + + let res = "bottom_degrees,location,state,surface_degrees,time\n\ + 50.4,santa_monica,CA,65.2,1568756160\n"; + check_response("query", response, StatusCode::OK, res).await; + + Ok(()) + } + + #[tokio::test] + async fn test_query_json() -> Result<()> { + let (client, server_url) = setup_test_data().await; + + // send a second line of data to demontrate how that works + let lp_data = "h2o_temperature,location=Boston,state=MA surface_degrees=50.2 1568756160"; + + // send write data + let bucket_name = "MyBucket"; + let org_name = "MyOrg"; + let response = client + .post(&format!( + "{}/api/v2/write?bucket={}&org={}", + server_url, bucket_name, org_name + )) + .body(lp_data) + .send() + .await; + + check_response("write", response, StatusCode::NO_CONTENT, "").await; + + // send query data + let response = client + .get(&format!( + "{}/iox/api/v1/databases/MyOrg_MyBucket/query?q={}&format=json", + server_url, "select%20*%20from%20h2o_temperature" + )) + .send() + .await; + + assert_eq!(get_content_type(&response), "application/json"); + + // Note two json records: one record on each line + let res = r#"[{"bottom_degrees":50.4,"location":"santa_monica","state":"CA","surface_degrees":65.2,"time":1568756160},{"location":"Boston","state":"MA","surface_degrees":50.2,"time":1568756160}]"#; + check_response("query", response, StatusCode::OK, res).await; + Ok(()) } @@ -987,7 +1057,7 @@ mod tests { "{}/api/v2/write?bucket={}&org={}", server_url, bucket_name, org_name )) - .header(header::CONTENT_ENCODING, "gzip") + .header(CONTENT_ENCODING, "gzip") .body(gzip_str(lp_data)) .send() .await; @@ -1241,6 +1311,19 @@ mod tests { assert_eq!(r4.segments.len(), 0); } + fn get_content_type(response: &Result) -> String { + if let Ok(response) = response { + response + .headers() + .get(CONTENT_TYPE) + .map(|v| v.to_str().unwrap()) + .unwrap_or("") + .to_string() + } else { + "".to_string() + } + } + /// checks a http response against expected results async fn check_response( description: &str, @@ -1313,4 +1396,59 @@ mod tests { collect(physical_plan).await.unwrap() } + + #[test] + fn query_params_format_default() { + // default to pretty format when not otherwise specified + assert_eq!( + serde_urlencoded::from_str("q=foo"), + Ok(QueryParams { + q: "foo".to_string(), + format: QueryOutputFormat::Pretty + }) + ); + } + + #[test] + fn query_params_format_pretty() { + assert_eq!( + serde_urlencoded::from_str("q=foo&format=pretty"), + Ok(QueryParams { + q: "foo".to_string(), + format: QueryOutputFormat::Pretty + }) + ); + } + + #[test] + fn query_params_format_csv() { + assert_eq!( + serde_urlencoded::from_str("q=foo&format=csv"), + Ok(QueryParams { + q: "foo".to_string(), + format: QueryOutputFormat::CSV + }) + ); + } + + #[test] + fn query_params_format_json() { + assert_eq!( + serde_urlencoded::from_str("q=foo&format=json"), + Ok(QueryParams { + q: "foo".to_string(), + format: QueryOutputFormat::JSON + }) + ); + } + + #[test] + fn query_params_bad_format() { + assert_eq!( + serde_urlencoded::from_str::("q=foo&format=jsob") + .unwrap_err() + .to_string(), + "unknown variant `jsob`, expected one of `pretty`, `csv`, `json`" + ); + } } diff --git a/src/influxdb_ioxd/http/format.rs b/src/influxdb_ioxd/http/format.rs new file mode 100644 index 0000000000..0c459c026e --- /dev/null +++ b/src/influxdb_ioxd/http/format.rs @@ -0,0 +1,242 @@ +//! Output formatting utilities for query endpoint + +use serde::Deserialize; +use snafu::{ResultExt, Snafu}; +use std::io::Write; + +use serde_json::Value; + +use arrow_deps::arrow::{ + self, csv::WriterBuilder, error::ArrowError, json::writer::record_batches_to_json_rows, + record_batch::RecordBatch, +}; + +#[derive(Debug, Snafu)] +pub enum Error { + #[snafu(display("Arrow pretty printing error: {}", source))] + PrettyArrow { source: ArrowError }, + + #[snafu(display("Arrow csv printing error: {}", source))] + CsvArrow { source: ArrowError }, + + #[snafu(display("Arrow json printing error: {}", source))] + JsonArrow { source: ArrowError }, + + #[snafu(display("Json conversion error: {}", source))] + JsonConversion { source: serde_json::Error }, + + #[snafu(display("IO error during Json conversion: {}", source))] + JsonWrite { source: std::io::Error }, + + #[snafu(display("Error converting CSV output to UTF-8: {}", source))] + CsvUtf8 { source: std::string::FromUtf8Error }, + + #[snafu(display("Error converting JSON output to UTF-8: {}", source))] + JsonUtf8 { source: std::string::FromUtf8Error }, +} +type Result = std::result::Result; + +#[derive(Deserialize, Debug, Copy, Clone, PartialEq)] +/// Requested output format for the query endpoint +pub enum QueryOutputFormat { + /// Arrow pretty printer format (default) + #[serde(rename = "pretty")] + Pretty, + /// Comma separated values + #[serde(rename = "csv")] + CSV, + /// Arrow JSON format + #[serde(rename = "json")] + JSON, +} + +impl Default for QueryOutputFormat { + fn default() -> Self { + Self::Pretty + } +} + +impl QueryOutputFormat { + /// Return the content type of the relevant format + pub fn content_type(&self) -> &'static str { + match self { + Self::Pretty => "text/plain", + Self::CSV => "text/csv", + Self::JSON => "application/json", + } + } +} + +impl QueryOutputFormat { + /// Format the [`RecordBatch`]es into a String in one of the + /// following formats: + /// + /// Pretty: + /// ```text + /// +----------------+--------------+-------+-----------------+------------+ + /// | bottom_degrees | location | state | surface_degrees | time | + /// +----------------+--------------+-------+-----------------+------------+ + /// | 50.4 | santa_monica | CA | 65.2 | 1568756160 | + /// +----------------+--------------+-------+-----------------+------------+ + /// ``` + /// + /// CSV: + /// ```text + /// bottom_degrees,location,state,surface_degrees,time + /// 50.4,santa_monica,CA,65.2,1568756160 + /// ``` + /// + /// JSON: + /// + /// Example (newline + whitespace added for clarity): + /// ```text + /// [ + /// {"bottom_degrees":50.4,"location":"santa_monica","state":"CA","surface_degrees":65.2,"time":1568756160}, + /// {"location":"Boston","state":"MA","surface_degrees":50.2,"time":1568756160} + /// ] + /// ``` + pub fn format(&self, batches: &[RecordBatch]) -> Result { + match self { + Self::Pretty => batches_to_pretty(&batches), + Self::CSV => batches_to_csv(&batches), + Self::JSON => batches_to_json(&batches), + } + } +} + +fn batches_to_pretty(batches: &[RecordBatch]) -> Result { + arrow::util::pretty::pretty_format_batches(batches).context(PrettyArrow) +} + +fn batches_to_csv(batches: &[RecordBatch]) -> Result { + let mut bytes = vec![]; + + { + let mut writer = WriterBuilder::new().has_headers(true).build(&mut bytes); + + for batch in batches { + writer.write(batch).context(CsvArrow)?; + } + } + let csv = String::from_utf8(bytes).context(CsvUtf8)?; + Ok(csv) +} + +fn batches_to_json(batches: &[RecordBatch]) -> Result { + let mut bytes = vec![]; + + { + let mut writer = JsonArrayWriter::new(&mut bytes); + writer.write_batches(batches)?; + writer.finish()?; + } + + let json = String::from_utf8(bytes).context(JsonUtf8)?; + + Ok(json) +} + +/// Writes out well formed JSON arays in a streaming fashion +/// +/// [{"foo": "bar"}, {"foo": "baz"}] +/// +/// This is based on the arrow JSON writer (json::writer::Writer) +/// +/// TODO contribute this back to arrow: https://issues.apache.org/jira/browse/ARROW-11773 +struct JsonArrayWriter +where + W: Write, +{ + started: bool, + finished: bool, + writer: W, +} + +impl JsonArrayWriter +where + W: Write, +{ + fn new(writer: W) -> Self { + Self { + writer, + started: false, + finished: false, + } + } + + /// Consume self and return the inner writer + #[cfg(test)] + pub fn into_inner(self) -> W { + self.writer + } + + pub fn write_row(&mut self, row: &Value) -> Result<()> { + if !self.started { + self.writer.write_all(b"[").context(JsonWrite)?; + self.started = true; + } else { + self.writer.write_all(b",").context(JsonWrite)?; + } + self.writer + .write_all(&serde_json::to_vec(row).context(JsonConversion)?) + .context(JsonWrite)?; + Ok(()) + } + + pub fn write_batches(&mut self, batches: &[RecordBatch]) -> Result<()> { + for row in record_batches_to_json_rows(batches) { + self.write_row(&Value::Object(row))?; + } + Ok(()) + } + + /// tell the writer there are is no more data to come so it can + /// write the final `'['` + pub fn finish(&mut self) -> Result<()> { + if self.started && !self.finished { + self.writer.write_all(b"]").context(JsonWrite)?; + self.finished = true; + } + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use serde_json::json; + + use super::*; + + #[test] + fn json_writer_empty() { + let mut writer = JsonArrayWriter::new(vec![] as Vec); + writer.finish().unwrap(); + assert_eq!(String::from_utf8(writer.into_inner()).unwrap(), ""); + } + + #[test] + fn json_writer_one_row() { + let mut writer = JsonArrayWriter::new(vec![] as Vec); + let v = json!({ "an": "object" }); + writer.write_row(&v).unwrap(); + writer.finish().unwrap(); + assert_eq!( + String::from_utf8(writer.into_inner()).unwrap(), + r#"[{"an":"object"}]"# + ); + } + + #[test] + fn json_writer_two_rows() { + let mut writer = JsonArrayWriter::new(vec![] as Vec); + let v = json!({ "an": "object" }); + writer.write_row(&v).unwrap(); + let v = json!({ "another": "object" }); + writer.write_row(&v).unwrap(); + writer.finish().unwrap(); + assert_eq!( + String::from_utf8(writer.into_inner()).unwrap(), + r#"[{"an":"object"},{"another":"object"}]"# + ); + } +} diff --git a/tests/end-to-end.rs b/tests/end-to-end.rs index 76f45430eb..bfc16f3a40 100644 --- a/tests/end-to-end.rs +++ b/tests/end-to-end.rs @@ -48,7 +48,7 @@ const HTTP_BIND_ADDR: &str = http_bind_addr!(); const GRPC_BIND_ADDR: &str = grpc_bind_addr!(); const HTTP_BASE: &str = concat!("http://", http_bind_addr!()); -const API_BASE: &str = concat!("http://", http_bind_addr!(), "/api/v2"); +const IOX_API_V1_BASE: &str = concat!("http://", http_bind_addr!(), "/iox/api/v1"); const GRPC_URL_BASE: &str = concat!("http://", grpc_bind_addr!(), "/"); const TOKEN: &str = "InfluxDB IOx doesn't have authentication yet"; diff --git a/tests/end_to_end_cases/read_api.rs b/tests/end_to_end_cases/read_api.rs index 5f224ace23..8a05c1f062 100644 --- a/tests/end_to_end_cases/read_api.rs +++ b/tests/end_to_end_cases/read_api.rs @@ -1,4 +1,4 @@ -use crate::{Scenario, API_BASE}; +use crate::{Scenario, IOX_API_V1_BASE}; pub async fn test( client: &reqwest::Client, @@ -6,7 +6,7 @@ pub async fn test( sql_query: &str, expected_read_data: &[String], ) { - let text = read_data_as_sql(&client, "/read", scenario, sql_query).await; + let text = read_data_as_sql(&client, scenario, sql_query).await; assert_eq!( text, expected_read_data, @@ -17,18 +17,15 @@ pub async fn test( async fn read_data_as_sql( client: &reqwest::Client, - path: &str, scenario: &Scenario, sql_query: &str, ) -> Vec { - let url = format!("{}{}", API_BASE, path); + let db_name = format!("{}_{}", scenario.org_id_str(), scenario.bucket_id_str()); + let path = format!("/databases/{}/query", db_name); + let url = format!("{}{}", IOX_API_V1_BASE, path); let lines = client .get(&url) - .query(&[ - ("bucket", scenario.bucket_id_str()), - ("org", scenario.org_id_str()), - ("sql_query", sql_query), - ]) + .query(&[("q", sql_query)]) .send() .await .unwrap() From c7343a4acf960bd8fdc83c41ca9c1e66e31991e6 Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Fri, 26 Feb 2021 14:10:28 -0500 Subject: [PATCH 22/22] feat: Add iterators over different types of columns in Schema (#879) * feat: Add iterators by InfluxColumnType * fix: Update data_types/src/schema.rs Implement PR suggestion Co-authored-by: Edd Robinson * refactor: Remove unecessary code Co-authored-by: Edd Robinson Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --- data_types/src/schema.rs | 129 +++++++++++++++++++++++++++++++++++---- 1 file changed, 117 insertions(+), 12 deletions(-) diff --git a/data_types/src/schema.rs b/data_types/src/schema.rs index 5d1b89a7cc..c22979bb11 100644 --- a/data_types/src/schema.rs +++ b/data_types/src/schema.rs @@ -299,10 +299,44 @@ impl Schema { /// Returns an iterator of (Option, &Field) for /// all the columns of this schema, in order pub fn iter(&self) -> SchemaIter<'_> { - SchemaIter { - schema: self, - idx: 0, - } + SchemaIter::new(self) + } + + /// Returns an iterator of `&Field` for all the tag columns of + /// this schema, in order + pub fn tags_iter(&self) -> impl Iterator { + self.iter().filter_map(|(influx_column_type, field)| { + if matches!(influx_column_type, Some(InfluxColumnType::Tag)) { + Some(field) + } else { + None + } + }) + } + + /// Returns an iterator of `&Field` for all the field columns of + /// this schema, in order + pub fn fields_iter(&self) -> impl Iterator { + self.iter().filter_map(|(influx_column_type, field)| { + if matches!(influx_column_type, Some(InfluxColumnType::Field(_))) { + Some(field) + } else { + None + } + }) + } + + /// Returns an iterator of `&Field` for all the timestamp columns + /// of this schema, in order. At the time of writing there should + /// be only one or 0 such columns + pub fn time_iter(&self) -> impl Iterator { + self.iter().filter_map(|(influx_column_type, field)| { + if matches!(influx_column_type, Some(InfluxColumnType::Timestamp)) { + Some(field) + } else { + None + } + }) } /// Merges any new columns from new_schema, consuming self. If the @@ -573,6 +607,12 @@ pub struct SchemaIter<'a> { idx: usize, } +impl<'a> SchemaIter<'a> { + fn new(schema: &'a Schema) -> Self { + Self { schema, idx: 0 } + } +} + impl<'a> fmt::Debug for SchemaIter<'a> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "SchemaIter<{}>", self.idx) @@ -829,15 +869,47 @@ mod test { } } + /// Build an empty iterator + fn empty_schema() -> Schema { + SchemaBuilder::new().build().unwrap() + } + + #[test] + fn test_iter_empty() { + assert_eq!(empty_schema().iter().count(), 0); + } + + #[test] + fn test_tags_iter_empty() { + assert_eq!(empty_schema().tags_iter().count(), 0); + } + + #[test] + fn test_fields_iter_empty() { + assert_eq!(empty_schema().fields_iter().count(), 0); + } + + #[test] + fn test_time_iter_empty() { + assert_eq!(empty_schema().time_iter().count(), 0); + } + + /// Build a schema for testing iterators + fn iter_schema() -> Schema { + SchemaBuilder::new() + .influx_field("field1", Float) + .tag("tag1") + .timestamp() + .influx_field("field2", String) + .influx_field("field3", String) + .tag("tag2") + .build() + .unwrap() + } + #[test] fn test_iter() { - let schema = SchemaBuilder::new() - .influx_field("the_field", String) - .tag("the_tag") - .timestamp() - .measurement("the_measurement") - .build() - .unwrap(); + let schema = iter_schema(); // test schema iterator and field accessor match up for (i, (iter_col_type, iter_field)) in schema.iter().enumerate() { @@ -845,7 +917,40 @@ mod test { assert_eq!(iter_col_type, col_type); assert_eq!(iter_field, field); } - assert_eq!(schema.iter().count(), 3); + assert_eq!(schema.iter().count(), 6); + } + + #[test] + fn test_tags_iter() { + let schema = iter_schema(); + + let mut iter = schema.tags_iter(); + assert_eq!(iter.next().unwrap().name(), "tag1"); + assert_eq!(iter.next().unwrap().name(), "tag2"); + assert_eq!(iter.next(), None); + assert_eq!(iter.next(), None); + } + + #[test] + fn test_fields_iter() { + let schema = iter_schema(); + + let mut iter = schema.fields_iter(); + assert_eq!(iter.next().unwrap().name(), "field1"); + assert_eq!(iter.next().unwrap().name(), "field2"); + assert_eq!(iter.next().unwrap().name(), "field3"); + assert_eq!(iter.next(), None); + assert_eq!(iter.next(), None); + } + + #[test] + fn test_time_iter() { + let schema = iter_schema(); + + let mut iter = schema.time_iter(); + assert_eq!(iter.next().unwrap().name(), "time"); + assert_eq!(iter.next(), None); + assert_eq!(iter.next(), None); } #[test]