feat: expose all tables via flight sql #571

nathanielc · 2024-10-28T21:40:21Z

This change moves the OLAP aggregation logic within ceramic-one. This means that all tables are available via Flight SQL endpoint.

TODO:

Manually test query command

stbrody · 2024-10-29T20:10:01Z

Going to wait to review until CI is green

nathanielc · 2024-10-31T19:25:22Z

@stbrody Ready for review now. Turns out there was a subtle issue with the federation used with the query command. So instead of trying to fix it I have changed the query command to be a simple client of the FlightSQL endpoint instead of full featured REPL

stbrody

a few small comments but generally LGTM.

One question: where does this leave folks who want to build their own custom aggregators? Can they still do so out-of-process by consuming the conclusion feed over the network? Does this PR make this more difficult?

stbrody · 2024-11-05T20:28:43Z

one/src/daemon.rs

+        requires = "flight_sql_bind_address",
+        env = "CERAMIC_ONE_AGGREGATOR"
+    )]
+    aggregator: Option<bool>,


I'm trying to think if this should be a state enum instead of a boolean, but I'm having a hard time thinking about what other modes the aggregator might have in the future. I guess since I can't think of anything than just a bool is probably fine

The plan is also to remove this boolean once we make it always on. We will likely need some form of config for which parts of the pipeline to enable. That will likely be a net new config option once we have move than one stage.

should the name of the flag be enable_aggregator maybe?

stbrody · 2024-11-05T20:29:45Z

one/src/daemon.rs

+    #[arg(
+        long,
+        requires = "experimental_features",
+        env = "CERAMIC_ONE_S3_BUCKET"


in the future the data files might live in a different bucket than the aggregator output right? So we might want this name to indicate that this specifically for the result of the aggregated state that we are exporting.

I am anticipating a single bucket for all the data. A single bucket and handle many tables. I don't want to complicate the API by expecting a different bucket for each table until that need arises in product discussions.

I'll update the comment to reflect this intention

stbrody · 2024-11-05T20:30:05Z

one/src/daemon.rs

+    aggregator: Option<bool>,
+
+    /// S3 bucket name.
+    /// When configured the aggregator will support storing data in S3 compatible object stores.


will support? or will actually do so?

stbrody · 2024-11-05T20:30:33Z

one/src/daemon.rs

@@ -469,7 +497,7 @@ pub async fn run(opts: DaemonOpts) -> Result<()> {
        .await
        .map_err(|e| {
            anyhow!(
-                "Failed to start libp2p server using addresses: {}. {}",
+                "Failed to start linebp2p server using addresses: {}. {}",


stbrody · 2024-11-05T20:31:29Z

one/src/daemon.rs

+    /// Enable the aggregator, requires Flight SQL to be enabled
+    #[arg(
+        long,
+        requires = "flight_sql_bind_address",


also requires the s3 bucket?

stbrody · 2024-11-05T20:32:14Z

one/src/daemon.rs

-                let _ = shutdown_signal.recv().await;
+        let bucket = opts
+            .s3_bucket
+            .ok_or_else(|| anyhow!("s3_bucket option is required when exposing flight sql"))?;


should this be required? What if you just want the run the aggregation locally and query out the aggregated state feed?

Today its required as there is no mechanism for storing the state locally. We should likely add that for dev purposes but that is a separate issue.

nathanielc · 2024-11-06T20:52:26Z

One question: where does this leave folks who want to build their own custom aggregators? Can they still do so out-of-process by consuming the conclusion feed over the network? Does this PR make this more difficult?

This does not make it more difficult. Consumers can still build their own aggregators out of band. In fact this makes is easier as all the data for any stage in the pipeline can be found in a single location.

This change moves the OLAP aggregation logic within ceramic-one. This means that all tables are available via Flight SQL endpoint.

The REPL query command using federation had many gotchas that were hard to predict. For example using a count query would confuse the federation optimizer rules. This changes the query command to be a plain (non REPL) client of the FlightSQL endpoint.

stbrody · 2024-11-06T21:06:44Z

This does not make it more difficult. Consumers can still build their own aggregators out of band. In fact this makes is easier as all the data for any stage in the pipeline can be found in a single location.

This does remove all the tooling for building an external validator though right? Now someone would need to not just make their specific aggregator, but also a build process for a standalone aggregator binary

nathanielc · 2024-11-06T21:29:01Z

This does remove all the tooling for building an external validator though right? Now someone would need to not just make their specific aggregator, but also a build process for a standalone aggregator binary

That was true before however they could have copied the olap binary. But it was quite small and didn't have anything interesting to preserve. It was a blind copy of the ceramic-one binary setup.

nathanielc requested review from stbrody and a team as code owners October 28, 2024 21:40

nathanielc requested review from smrz2001 and removed request for a team October 28, 2024 21:40

nathanielc force-pushed the feat/single-binary-olap branch from 9e30ee6 to 3deb641 Compare October 28, 2024 21:41

nathanielc had a problem deploying to github-tests-2024 October 28, 2024 22:02 — with GitHub Actions Failure

nathanielc temporarily deployed to github-tests-2024 October 30, 2024 21:40 — with GitHub Actions Inactive

nathanielc force-pushed the feat/single-binary-olap branch from 2f6e6c6 to 2cf7a61 Compare November 5, 2024 20:15

nathanielc temporarily deployed to github-tests-2024 November 5, 2024 20:34 — with GitHub Actions Inactive

stbrody approved these changes Nov 5, 2024

View reviewed changes

nathanielc added 4 commits November 6, 2024 14:05

feat: expose all tables via flight sql

92049cd

This change moves the OLAP aggregation logic within ceramic-one. This means that all tables are available via Flight SQL endpoint.

fix: cleanup deps

2141efe

fix: update cli flags descriptions

e90b766

nathanielc force-pushed the feat/single-binary-olap branch from 6b18092 to e90b766 Compare November 6, 2024 21:05

fix: update cargo.lock

5b41edd

nathanielc temporarily deployed to github-tests-2024 November 6, 2024 21:25 — with GitHub Actions Inactive

nathanielc enabled auto-merge November 6, 2024 21:27

nathanielc added this pull request to the merge queue Nov 6, 2024

Merged via the queue into main with commit 7817be9 Nov 6, 2024
5 checks passed

nathanielc deleted the feat/single-binary-olap branch November 6, 2024 22:06

This was referenced Nov 7, 2024

chore: version v0.41.2 #578

Closed

chore: version v0.42.0 #579

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: expose all tables via flight sql #571

feat: expose all tables via flight sql #571

nathanielc commented Oct 28, 2024 •

edited

Loading

stbrody commented Oct 29, 2024

nathanielc commented Oct 31, 2024

stbrody left a comment

stbrody Nov 5, 2024

nathanielc Nov 6, 2024

stbrody Nov 6, 2024

stbrody Nov 5, 2024

nathanielc Nov 6, 2024

nathanielc Nov 6, 2024

stbrody Nov 5, 2024

stbrody Nov 5, 2024

stbrody Nov 5, 2024

stbrody Nov 5, 2024

nathanielc Nov 6, 2024

nathanielc commented Nov 6, 2024

stbrody commented Nov 6, 2024

nathanielc commented Nov 6, 2024

feat: expose all tables via flight sql #571

feat: expose all tables via flight sql #571

Conversation

nathanielc commented Oct 28, 2024 • edited Loading

stbrody commented Oct 29, 2024

nathanielc commented Oct 31, 2024

stbrody left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nathanielc commented Nov 6, 2024

stbrody commented Nov 6, 2024

nathanielc commented Nov 6, 2024

nathanielc commented Oct 28, 2024 •

edited

Loading