Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat: add system.parquet_files table #25225

Merged
merged 3 commits into from
Aug 8, 2024

Conversation

hiltontj
Copy link
Contributor

@hiltontj hiltontj commented Aug 7, 2024

Closes #24988

(Supplants #25002)

This extends the system tables available with a new parquet_files table which will list the parquet files associated with a given table in a database.

Example

Queries to system.parquet_files must provide a table_name predicate to specify the table name of interest, like so:

SELECT * FROM system.parquet_files WHERE table_name = 'cpu'

This will produce a result like this:

+------------+-----------------------------------------------------+------------+-----------+----------+----------+
| table_name | path                                                | size_bytes | row_count | min_time | max_time |
+------------+-----------------------------------------------------+------------+-----------+----------+----------+
| cpu        | dbs/test_db/cpu/1970-01-01/00-00/0000000003.parquet | 2147       | 3         | 0        | 20       |
| cpu        | dbs/test_db/cpu/1970-01-01/00-00/0000000006.parquet | 2147       | 3         | 30       | 50       |
| cpu        | dbs/test_db/cpu/1970-01-01/00-00/0000000009.parquet | 2147       | 3         | 60       | 80       |
+------------+-----------------------------------------------------+------------+-----------+----------+----------+

Other Details

  • The parquet file information is accessed through the QueryableBuffer, which is exposed as pub in this PR.
  • Two tests were added to check success and failure modes of the new system table query.
  • The Persister trait had its associated Error type removed. This was somewhat of a consequence of how I initially implemented this change, but I felt cleaned the code up a bit, so I kept it in the commit. If you want it removed to another PR I can do that.

@hiltontj hiltontj self-assigned this Aug 7, 2024
@hiltontj hiltontj added the v3 label Aug 7, 2024
This extends the system tables available with a new `parquet_files` table
which will list the parquet files associated with a given table in a
database.

Queries to system.parquet_files must provide a table_name predicate to
specify the table name of interest.

The files are accessed through the QueryableBuffer.

In addition, a test was added to check success and failure modes of the
new system table query.

Finally, the Persister trait had its associated error type removed. This
was somewhat of a consequence of how I initially implemented this change,
but I felt cleaned the code up a bit, so I kept it in the commit.
This also fixed a test that broke after merge.
Copy link
Member

@pauldix pauldix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two small requests, but otherwise looks great

level_0_duration,
Arc::clone(&executor),
WalConfig {
level_0_duration: Duration::from_millis(100),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm about to change this in a follow on PR to use the Level0Duration type, which is only valid for 1m, 5m, or 10m durations. Can you update this test to use one of those?

Copy link
Contributor Author

@hiltontj hiltontj Aug 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is addressed in ff6e9a4 by using a 60 second duration. So switching to a Level0Duration::1m should be a straightforward change.

"| public | information_schema | schemata | VIEW |",
"| public | information_schema | tables | VIEW |",
"| public | information_schema | views | VIEW |",
"| public | iox | cpu | BASE TABLE |",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated to this PR, but we should update these tables to be influxdb3 tables. I logged #25227 to track.

@@ -84,6 +85,9 @@ pub trait Bufferer: Debug + Send + Sync + 'static {

/// Returns the catalog
fn catalog(&self) -> Arc<catalog::Catalog>;

/// Returns the queryable buffer
fn queryable_buffer(&self) -> Arc<QueryableBuffer>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than giving access directly to the queryable buffer, can you put a function on this trait to get access to the parquet files? I think this will work more cleanly for when we have multiple queryable buffers in Pro and it will implement this trait to hide that fact.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, that makes sense - I'll give it a shot!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I addressed this in ff6e9a4 by giving a Arc<dyn WriteBuffer> to the system table provider, which now has a method that produces parquet files for a given db/table.

Some Arc 🔥 was involved, which spread throughout the codebase.


One other thing I could think would be to produce an error if an invalid table name is provided. The underlying function on the QueryableBuffer that gives the parquet files is called in the query path where the db/table name are validated with the catalog. So, if an invalid table name is passed, it will produce an empty record set, vs. an error.

I can work that into this PR or create a follow-on issue to address that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe save for a follow on

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened #25229 to track.

The system table provider uses the buffer directly, so that the implementation
of files being retrieved is obscured. Some Arcs were involved.

A test was updated to use a 1m Level 0 Duration.
Copy link
Member

@pauldix pauldix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@hiltontj hiltontj merged commit 7474c0b into main Aug 8, 2024
12 checks passed
@hiltontj hiltontj deleted the hiltontj/system-parquet-files-table branch August 8, 2024 12:46
mgattozzi added a commit that referenced this pull request Sep 5, 2024
* fix: wal skip persist and notify if empty buffer (#25211)

* fix: wal skip persist and notify if empty buffer

This fixes the WAL so that it will skip persisting a file and notifying the file notifier if the wal buffer is empty.

* fix: fix last cache persist test

* fix: make ParquetChunk fields and mod chunk pub (#25219)

* fix: make ParquetChunk fields and mod chunk pub

This doesn't affect anything in the OSS version, but these changes are
needed for Pro as part of our compactor work.

* fix: cargo deny failure

* fix: query bugs with buffer (#25213)

* fix: query bugs with buffer

This fixes three different bugs with the buffer. First was that aggregations would fail because projection was pushed down to the in-buffer data that de-duplication needs to be called on. The test in influxdb3/tests/server/query.rs catches that.

I also added a test in write_buffer/mod.rs to ensure that data is correctly queryable when combining with different states: only data in buffer, only data in parquet files, and data across both. This showed two bugs, one where the parquet data was being doubled up (parquet chunks were being created in write buffer mod and in queryable buffer. The second was that the timestamp min max on table buffer would panic if the buffer was empty.

* refactor: PR feedback

* fix: fix wal replay and buffer snapshot

Fixes two problems uncovered by adding to the write_buffer/mod.rs test. Ensures we can replay wal data and that snapshots work properly with replayed data.

* fix: run cargo update to fix audit

* feat: use host identifier prefix in object store paths (#25224)

This enforces the use of a host identifier prefix in all object store
paths (currently, for parquet files, catalog files, and snapshot files).

The persister retains the host identifier prefix, and uses it when
constructing paths.

The WalObjectStore also holds the host identifier prefix, so that it can
use it when saving and loading WAL files.

The influxdb3 binary requires a new argument 'host-id' to be passed that
is used to specify the prefix.

* feat: add `system.parquet_files` table (#25225)

This extends the system tables available with a new `parquet_files` table
which will list the parquet files associated with a given table in a
database.

Queries to system.parquet_files must provide a table_name predicate to
specify the table name of interest.

The files are accessed through the QueryableBuffer.

In addition, a test was added to check success and failure modes of the
new system table query.

Finally, the Persister trait had its associated error type removed. This
was somewhat of a consequence of how I initially implemented this change,
but I felt cleaned the code up a bit, so I kept it in the commit.

* fix: un-pub QueryableBuffer and fix compile errors (#25230)

* refactor: Make Level0Duration part of WAL (#25228)

* refactor: Make Level0Duration part of WAL

I noticed this during some testing and cleanup with other PRs. The WAL had its own level_0_duration and the write buffer had a different one, which would cause some weird problems if they weren't the same. This refactors Level0Duration to be in the WAL and fixes up the tests.

As an added bonus, this surfaced a bug where multiple L0 blocks getting persisted in the same snapshot wasn't supported. So now snapshot details can have many files per table.

* fix: have persisted files always return in descending data time order

* fix: sort record batches for test verification

* fix: main (#25231)

* feat: Add last cache create/delete to WAL (#25233)

* feat: Add last cache create/delete to WAL

This moves the LastCacheDefinition into the WAL so that it can be serialized there. This ended up being a pretty large refactor to get the last cache creation to work through the WAL.

I think I also stumbled on a bug where the last cache wasn't getting initialized from the catalog on reboot so that it wouldn't actually end up caching values. The refactored last cache persistence test in write_buffer/mod.rs surfaced this.

Finally, I also had to update the WAL so that it would persist if there were only catalog updates and no writes.

Fixes #25203

* fix: typos

* feat: Catalog apply_catalog_batch only updates if new (#25236)

* feat: Catalog apply_catalog_batch only updates if new

This updates the Catalog so that when applying a catalog batch it only updates the inner catalog and bumps the sequence number and updated tracker if there are new updates in the batch. Also does validation that the catalog batch schema is compatible with any existing.

Closes #25205

* feat: only persist catalog when updated (#25238)

* chore: ignore sqlx rustsec advisory (#25252)

* feat: Add FileIndex type to influxdb3_index

This commit does two important things:

1. It creates a new influxdb3_index crate under influxdb3_pro to contain
   all indexing logic and types that we might create for influxdb3_pro
2. Creates our first index type the FileIndex which is part of #20

Note we're starting off with just file ids as this will let us set up
the logic for creating and working with the `FileIndex` inside of the
compactor first. Later we can add row groups as that logic is a bit more
complicated in nature.

The `FileIndex` contains methods to lookup, insert, and delete items
from the index as needed and an associated test to make sure it works as
expected.

Note that the `FileIndex` is meant to have one created for each database
table that has an index created for it. Later on when it's being
integrated into the compactor a `FileIndex` will be returned per
compaction of a given table. We'll later integrate this into the
`WriteBuffer` for querying as well as adding this to the WAL so that
indexes can be recreated as needed.

---------

Co-authored-by: Paul Dix <paul@pauldix.net>
Co-authored-by: Trevor Hilton <thilton@influxdata.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create the system.parquet_files provider
2 participants