Skip to content

Commit c479b40

Browse files
goffrieConvex, Inc.
authored and
Convex, Inc.
committed
Lower MAX_USER_MODULES to 4096 but make it actually work (#36478)
There were a couple issues: - Audit logging creates a ConvexArray with one entry per user module, which implies a hard limit of 8192 - Also, we would hit a TooManyReads error when replacing too many modules in one transaction. The former is fixed by lowering MAX_USER_MODULES to be safely within the limit - 4096 for now. This could be raised a little if necessary, but otherwise we would have to change the audit log schema. The latter is fixed by a somewhat hacky change: since we already read out all the modules from the database, we can read the `by_id` index instead of `by_creation_time` (which is used by full_table_scan). Then, the additional read ranges created by `Transaction::replace` will merge with the full-table range, instead of creating many small ones. GitOrigin-RevId: 70d43a529b669e59af76fbb87502e61cd265f340
1 parent 65b6205 commit c479b40

File tree

7 files changed

+236
-137
lines changed

7 files changed

+236
-137
lines changed

crates/application/src/deploy_config.rs

Lines changed: 123 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@ use std::{
33
BTreeMap,
44
BTreeSet,
55
},
6-
time::Duration,
6+
time::{
7+
Duration,
8+
Instant,
9+
},
710
};
811

912
use anyhow::Context;
@@ -29,16 +32,19 @@ use common::{
2932
Runtime,
3033
UnixTimestamp,
3134
},
35+
schemas::DatabaseSchema,
3236
types::{
3337
EnvVarName,
3438
EnvVarValue,
3539
ModuleEnvironment,
3640
NodeDependency,
3741
},
42+
version::Version,
3843
};
3944
use database::{
4045
BootstrapComponentsModel,
4146
IndexModel,
47+
OccRetryStats,
4248
Token,
4349
WriteSource,
4450
SCHEMAS_TABLE,
@@ -120,7 +126,26 @@ use value::{
120126
TableNamespace,
121127
};
122128

123-
use crate::Application;
129+
use crate::{
130+
Application,
131+
ApplyConfigArgs,
132+
ConfigMetadataAndSchema,
133+
};
134+
135+
pub struct PushAnalytics {
136+
pub config: ConfigMetadata,
137+
pub modules: Vec<ModuleConfig>,
138+
pub udf_server_version: Version,
139+
pub analyze_results: BTreeMap<CanonicalizedModulePath, AnalyzedModule>,
140+
pub schema: Option<DatabaseSchema>,
141+
}
142+
143+
pub struct PushMetrics {
144+
pub build_external_deps_time: Duration,
145+
pub upload_source_package_time: Duration,
146+
pub analyze_time: Duration,
147+
pub occ_stats: OccRetryStats,
148+
}
124149

125150
impl<RT: Runtime> Application<RT> {
126151
#[fastrace::trace]
@@ -622,6 +647,102 @@ impl<RT: Runtime> Application<RT> {
622647

623648
Ok(diff)
624649
}
650+
651+
/// N.B.: does not check auth
652+
pub async fn push_config_no_components(
653+
&self,
654+
identity: Identity,
655+
config_file: ConfigFile,
656+
modules: Vec<ModuleConfig>,
657+
udf_server_version: Version,
658+
schema_id: Option<String>,
659+
node_dependencies: Option<Vec<NodeDependencyJson>>,
660+
) -> anyhow::Result<(PushAnalytics, PushMetrics)> {
661+
let begin_build_external_deps = Instant::now();
662+
// Upload external node dependencies separately
663+
let external_deps_id_and_pkg = if let Some(deps) = node_dependencies
664+
&& !deps.is_empty()
665+
{
666+
let deps: Vec<_> = deps.into_iter().map(NodeDependency::from).collect();
667+
Some(self.build_external_node_deps(deps).await?)
668+
} else {
669+
None
670+
};
671+
let end_build_external_deps = Instant::now();
672+
let external_deps_pkg_size = external_deps_id_and_pkg
673+
.as_ref()
674+
.map(|(_, pkg)| pkg.package_size)
675+
.unwrap_or_default();
676+
677+
let source_package = self
678+
.upload_package(&modules, external_deps_id_and_pkg)
679+
.await?;
680+
let end_upload_source_package = Instant::now();
681+
// Verify that we have not exceeded the max zipped or unzipped file size
682+
let combined_pkg_size = source_package.package_size + external_deps_pkg_size;
683+
combined_pkg_size.verify_size()?;
684+
685+
let udf_config = UdfConfig {
686+
server_version: udf_server_version,
687+
// Generate a new seed and timestamp to be used at import time.
688+
import_phase_rng_seed: self.runtime.rng().random(),
689+
import_phase_unix_timestamp: self.runtime.unix_timestamp(),
690+
};
691+
let begin_analyze = Instant::now();
692+
// Note: This is not transactional with the rest of the deploy to avoid keeping
693+
// a transaction open for a long time.
694+
let mut tx = self.begin(Identity::system()).await?;
695+
let user_environment_variables = EnvironmentVariablesModel::new(&mut tx).get_all().await?;
696+
let system_env_var_overrides = system_env_var_overrides(&mut tx).await?;
697+
drop(tx);
698+
// Run analyze to make sure the new modules are valid.
699+
let (auth_module, analyze_results) = self
700+
.analyze_modules_with_auth_config(
701+
udf_config.clone(),
702+
modules.clone(),
703+
source_package.clone(),
704+
user_environment_variables,
705+
system_env_var_overrides,
706+
)
707+
.await?;
708+
let end_analyze = Instant::now();
709+
let (
710+
ConfigMetadataAndSchema {
711+
config_metadata,
712+
schema,
713+
},
714+
occ_stats,
715+
) = self
716+
.apply_config_with_retries(
717+
identity.clone(),
718+
ApplyConfigArgs {
719+
auth_module,
720+
config_file,
721+
schema_id,
722+
modules: modules.clone(),
723+
udf_config: udf_config.clone(),
724+
source_package,
725+
analyze_results: analyze_results.clone(),
726+
},
727+
)
728+
.await?;
729+
730+
Ok((
731+
PushAnalytics {
732+
config: config_metadata,
733+
modules,
734+
udf_server_version: udf_config.server_version,
735+
analyze_results,
736+
schema,
737+
},
738+
PushMetrics {
739+
build_external_deps_time: end_build_external_deps - begin_build_external_deps,
740+
upload_source_package_time: end_upload_source_package - end_build_external_deps,
741+
analyze_time: end_analyze - begin_analyze,
742+
occ_stats,
743+
},
744+
))
745+
}
625746
}
626747

627748
struct ApplicationInitializerEvaluator<'a, RT: Runtime> {

crates/application/src/tests/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ mod http_action;
1010
mod indexes;
1111
mod mutation;
1212
mod occ_retries;
13+
mod push;
1314
mod query_cache;
1415
mod returns_validation;
1516
mod scheduled_jobs;

crates/application/src/tests/push.rs

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
use common::knobs::MAX_USER_MODULES;
2+
use keybroker::Identity;
3+
use model::config::types::ConfigFile;
4+
use runtime::testing::TestRuntime;
5+
6+
use crate::{
7+
deploy_config::{
8+
AppDefinitionConfigJson,
9+
ModuleJson,
10+
StartPushRequest,
11+
},
12+
test_helpers::ApplicationTestExt as _,
13+
Application,
14+
};
15+
16+
fn make_modules() -> Vec<ModuleJson> {
17+
let mut functions: Vec<_> = (0..*MAX_USER_MODULES)
18+
.map(|i| ModuleJson {
19+
environment: None,
20+
source_map: None,
21+
path: format!("mod{i}.js"),
22+
source: format!("// {i}"),
23+
})
24+
.collect();
25+
functions.extend((0..*MAX_USER_MODULES).map(|i| ModuleJson {
26+
environment: None,
27+
source_map: None,
28+
path: format!("_deps/mod{i}.js"),
29+
source: format!("// dep {i}"),
30+
}));
31+
functions
32+
}
33+
34+
#[convex_macro::test_runtime]
35+
async fn test_max_size_push(rt: TestRuntime) -> anyhow::Result<()> {
36+
let application = Application::new_for_tests(&rt).await?;
37+
for _ in 0..2 {
38+
application
39+
.run_test_push(StartPushRequest {
40+
admin_key: "".into(),
41+
dry_run: false,
42+
functions: "convex/".into(),
43+
app_definition: AppDefinitionConfigJson {
44+
definition: None,
45+
dependencies: vec![],
46+
schema: None,
47+
functions: make_modules(),
48+
udf_server_version: "1.3939.3939".into(),
49+
},
50+
component_definitions: vec![],
51+
node_dependencies: vec![],
52+
})
53+
.await?;
54+
}
55+
Ok(())
56+
}
57+
58+
#[convex_macro::test_runtime]
59+
async fn test_max_size_push_no_components(rt: TestRuntime) -> anyhow::Result<()> {
60+
let application = Application::new_for_tests(&rt).await?;
61+
for _ in 0..2 {
62+
application
63+
.push_config_no_components(
64+
Identity::system(),
65+
ConfigFile {
66+
auth_info: None,
67+
functions: "convex/".into(),
68+
},
69+
make_modules()
70+
.into_iter()
71+
.map(|m| m.try_into().unwrap())
72+
.collect(),
73+
"1.3939.3939".parse().unwrap(),
74+
None,
75+
None,
76+
)
77+
.await?;
78+
}
79+
Ok(())
80+
}

crates/common/src/knobs.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1151,7 +1151,7 @@ pub static MAX_ECHO_BYTES: LazyLock<usize> =
11511151

11521152
/// The limit on the number of user modules in a push bundle.
11531153
pub static MAX_USER_MODULES: LazyLock<usize> =
1154-
LazyLock::new(|| env_config("MAX_USER_MODULES", 10000));
1154+
LazyLock::new(|| env_config("MAX_USER_MODULES", 4096));
11551155

11561156
/// Percentage of request traces that should sampled.
11571157
///

crates/isolate/src/environment/analyze.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ impl AnalyzeEnvironment {
325325
let module_config = self
326326
.modules
327327
.get(path)
328-
.ok_or(anyhow!("could not find module config in environment"))?;
328+
.context("could not find module config in environment")?;
329329
let source_map = module_config
330330
.source_map
331331
.as_ref()

0 commit comments

Comments
 (0)