Skip to content

Commit

Permalink
Don't store exports in system metadata (#30337)
Browse files Browse the repository at this point in the history
GitOrigin-RevId: b77f8a8f3c12edf69d4b59999539a202e0e92de5
  • Loading branch information
sujayakar authored and Convex, Inc. committed Oct 4, 2024
1 parent 96c5f6f commit 176a20d
Show file tree
Hide file tree
Showing 6 changed files with 93 additions and 38 deletions.
31 changes: 23 additions & 8 deletions crates/application/src/deploy_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ use model::{
ComponentDiff,
SchemaChange,
},
file_based_routing::add_file_based_routing,
file_based_routing::file_based_exports,
type_checking::{
CheckedComponent,
InitializerEvaluator,
Expand Down Expand Up @@ -166,7 +166,7 @@ impl<RT: Runtime> Application<RT> {
)
.await?;

let evaluated_components = self
let mut evaluated_components = self
.evaluate_components(
config,
&component_definition_packages,
Expand Down Expand Up @@ -203,6 +203,19 @@ impl<RT: Runtime> Application<RT> {
}
schema_change
};

// TODO(ENG-7533): Clean up exports from the start push response when we've
// updated clients to use `functions` directly.
for (path, definition) in evaluated_components.iter_mut() {
// We don't need to include exports for the root since we don't use codegen
// for the app's `api` object.
if path.is_root() {
continue;
}
anyhow::ensure!(definition.definition.exports.is_empty());
definition.definition.exports = file_based_exports(&definition.functions)?;
}

let resp = StartPushResponse {
environment_variables,
external_deps_id,
Expand Down Expand Up @@ -351,11 +364,6 @@ impl<RT: Runtime> Application<RT> {
},
);
}

// Add in file-based routing.
for definition in evaluated_components.values_mut() {
add_file_based_routing(definition)?;
}
Ok(evaluated_components)
}

Expand Down Expand Up @@ -491,7 +499,7 @@ impl<RT: Runtime> Application<RT> {
pub async fn finish_push(
&self,
identity: Identity,
start_push: StartPushResponse,
mut start_push: StartPushResponse,
) -> anyhow::Result<FinishPushDiff> {
// Download all source packages. We can remove this once we don't store source
// in the database.
Expand All @@ -506,6 +514,13 @@ impl<RT: Runtime> Application<RT> {
downloaded_source_packages.insert(definition_path.clone(), package);
}

// TODO(ENG-7533): Strip out exports from the `StartPushResponse` since we don't
// want to actually store it in the database. Remove this path once
// we've stopped sending exports down to the client.
for definition in start_push.analysis.values_mut() {
definition.definition.exports = BTreeMap::new();
}

let mut tx = self.begin(identity.clone()).await?;

// Validate that environment variables haven't changed since `start_push`.
Expand Down
21 changes: 15 additions & 6 deletions crates/database/src/bootstrap_model/components/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ use crate::{
SystemIndex,
SystemTable,
},
metrics,
ResolvedQuery,
Transaction,
COMPONENT_DEFINITIONS_TABLE,
Expand Down Expand Up @@ -274,11 +275,15 @@ impl<'a, RT: Runtime> BootstrapComponentsModel<'a, RT> {
ComponentDefinitionId::Child(id) => id,
};
let component_definition_doc_id = self.resolve_component_definition_id(internal_id)?;
self.tx
.get(component_definition_doc_id)
.await?
.map(TryInto::try_into)
.transpose()
let Some(doc) = self.tx.get(component_definition_doc_id).await? else {
return Ok(None);
};
let mut doc: ParsedDocument<ComponentDefinitionMetadata> = doc.try_into()?;
if !doc.exports.is_empty() {
metrics::log_nonempty_component_exports();
doc.exports = BTreeMap::new();
}
Ok(Some(doc))
}

pub async fn load_definition_metadata(
Expand Down Expand Up @@ -321,7 +326,11 @@ impl<'a, RT: Runtime> BootstrapComponentsModel<'a, RT> {
)?;
let mut definitions = BTreeMap::new();
while let Some(doc) = query.next(self.tx, None).await? {
let definition: ParsedDocument<ComponentDefinitionMetadata> = doc.try_into()?;
let mut definition: ParsedDocument<ComponentDefinitionMetadata> = doc.try_into()?;
if !definition.exports.is_empty() {
metrics::log_nonempty_component_exports();
definition.exports = BTreeMap::new();
}
anyhow::ensure!(definitions
.insert(definition.path.clone(), definition)
.is_none());
Expand Down
8 changes: 8 additions & 0 deletions crates/database/src/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1010,3 +1010,11 @@ pub mod vector {
CancelableTimer::new(&DATABASE_VECTOR_SEARCH_WITH_RETRIES_QUERY_SECONDS)
}
}

register_convex_counter!(
DATABASE_NONEMPTY_COMPONENT_EXPORTS_TOTAL,
"Nonempty component definition loaded from database"
);
pub fn log_nonempty_component_exports() {
log_counter(&DATABASE_NONEMPTY_COMPONENT_EXPORTS_TOTAL, 1);
}
23 changes: 16 additions & 7 deletions crates/model/src/components/file_based_routing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,22 @@ use common::{
components::Reference,
};
use errors::ErrorMetadata;
use sync_types::CanonicalizedUdfPath;
use sync_types::{
path::PathComponent,
CanonicalizedModulePath,
CanonicalizedUdfPath,
};

use super::types::EvaluatedComponentDefinition;
use crate::modules::module_versions::Visibility;
use crate::modules::module_versions::{
AnalyzedModule,
Visibility,
};

pub fn add_file_based_routing(evaluated: &mut EvaluatedComponentDefinition) -> anyhow::Result<()> {
for (module_path, module) in &evaluated.functions {
pub fn file_based_exports(
functions: &BTreeMap<CanonicalizedModulePath, AnalyzedModule>,
) -> anyhow::Result<BTreeMap<PathComponent, ComponentExport>> {
let mut exports = BTreeMap::new();
for (module_path, module) in functions {
let mut identifiers = vec![];
let stripped = module_path.clone().strip();

Expand All @@ -27,7 +36,7 @@ pub fn add_file_based_routing(evaluated: &mut EvaluatedComponentDefinition) -> a
path.push(function.name.clone().into());
let (last, prefix) = path.split_last().unwrap();

let mut current = &mut evaluated.definition.exports;
let mut current = &mut exports;
for identifier in prefix {
let current_node = current
.entry(identifier.clone())
Expand Down Expand Up @@ -60,5 +69,5 @@ pub fn add_file_based_routing(evaluated: &mut EvaluatedComponentDefinition) -> a
}
}
}
Ok(())
Ok(exports)
}
33 changes: 23 additions & 10 deletions crates/model/src/components/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ use database::{
Transaction,
};
use errors::ErrorMetadata;
use file_based_routing::file_based_exports;
use sync_types::{
path::PathComponent,
CanonicalizedUdfPath,
Expand Down Expand Up @@ -153,17 +154,32 @@ impl<'a, RT: Runtime> ComponentsModel<'a, RT> {
Ok(result)
}

pub async fn load_component_exports(
&mut self,
component_id: ComponentId,
) -> anyhow::Result<BTreeMap<PathComponent, ComponentExport>> {
let mut modules = BTreeMap::new();
for module in ModuleModel::new(self.tx)
.get_all_metadata(component_id)
.await?
{
let module = module.into_value();
let Some(analyze_result) = module.analyze_result else {
continue;
};
modules.insert(module.path, analyze_result);
}
file_based_exports(&modules)
}

#[async_recursion]
pub async fn resolve_export(
&mut self,
component_id: ComponentId,
attributes: &[PathComponent],
) -> anyhow::Result<Option<Resource>> {
let mut m = BootstrapComponentsModel::new(self.tx);
let definition_id = m.component_definition(component_id).await?;
let definition = m.load_definition_metadata(definition_id).await?;

let mut current = &definition.exports;
let exports = self.load_component_exports(component_id).await?;
let mut current = &exports;
let mut attribute_iter = attributes.iter();
while let Some(attribute) = attribute_iter.next() {
let Some(export) = current.get(attribute) else {
Expand Down Expand Up @@ -344,11 +360,8 @@ impl<'a, RT: Runtime> ComponentsModel<'a, RT> {
&mut self,
component_id: ComponentId,
) -> anyhow::Result<BTreeMap<Vec<PathComponent>, Resource>> {
let mut m = BootstrapComponentsModel::new(self.tx);
let definition_id = m.component_definition(component_id).await?;
let definition = m.load_definition_metadata(definition_id).await?;

let mut stack = vec![(vec![], &definition.exports)];
let exports = self.load_component_exports(component_id).await?;
let mut stack = vec![(vec![], &exports)];
let mut result = BTreeMap::new();
while let Some((path, internal_node)) = stack.pop() {
for (name, export) in internal_node {
Expand Down
15 changes: 8 additions & 7 deletions crates/model/src/components/type_checking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ use value::{
TableNamespace,
};

use super::types::EvaluatedComponentDefinition;
use super::{
file_based_routing::file_based_exports,
types::EvaluatedComponentDefinition,
};
use crate::{
modules::HTTP_MODULE_PATH,
virtual_system_mapping,
Expand Down Expand Up @@ -151,7 +154,7 @@ impl<'a> TypecheckContext<'a> {
}

// Finally, resolve our exports and build the component.
let component = builder.check_exports(&evaluated.definition.exports)?;
let component = builder.build_exports()?;

Ok(component)
}
Expand Down Expand Up @@ -323,11 +326,9 @@ impl<'a> CheckedComponentBuilder<'a> {
Ok(())
}

fn check_exports(
self,
exports: &BTreeMap<PathComponent, ComponentExport>,
) -> anyhow::Result<CheckedComponent> {
let exports = self.resolve_exports(exports)?;
fn build_exports(self) -> anyhow::Result<CheckedComponent> {
let exports = file_based_exports(&self.evaluated.functions)?;
let exports = self.resolve_exports(&exports)?;
Ok(CheckedComponent {
definition_path: self.definition_path.clone(),
component_path: self.component_path.clone(),
Expand Down

0 comments on commit 176a20d

Please sign in to comment.