Skip to content

Commit 214d6b9

Browse files
committed
Revert "Add support for starting/restarting without state in leader mode (#1111)"
This reverts commit fab1668.
1 parent f6afb83 commit 214d6b9

7 files changed

Lines changed: 22 additions & 123 deletions

File tree

crates/arroyo-api/queries/api_queries.sql

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,6 @@ UPDATE job_configs
183183
SET
184184
updated_at = :updated_at,
185185
updated_by = :updated_by,
186-
stop = 'none',
187186
restart_nonce = restart_nonce + 1,
188187
restart_mode = :mode,
189188
ignore_state_before_epoch = :ignore_state_before_epoch

crates/arroyo-api/src/pipelines.rs

Lines changed: 12 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ use crate::rest_utils::{
5252
use crate::types::public::{PipelineType, RestartMode, StopMode};
5353
use crate::udfs::build_udf;
5454
use crate::{connection_tables, to_micros};
55-
use arroyo_rpc::config::{JobControllerMode, config};
55+
use arroyo_rpc::config::config;
5656
use arroyo_rpc::errors::ErrorDomain;
5757
use arroyo_types::to_millis;
5858
use cornucopia_async::{Database, DatabaseSource};
@@ -868,44 +868,29 @@ pub async fn restart_pipeline(
868868
WithRejection(Json(req), _): WithRejection<Json<PipelineRestart>, ApiError>,
869869
) -> Result<Json<Pipeline>, ErrorResp> {
870870
let auth_data = authenticate(&state.database, bearer_auth).await?;
871-
872871
let db = state.database.client().await?;
873872

874-
let job = api_queries::fetch_get_pipeline_jobs(&db, &auth_data.organization_id, &id)
873+
let job_id = api_queries::fetch_get_pipeline_jobs(&db, &auth_data.organization_id, &id)
875874
.await?
876875
.into_iter()
877876
.next()
878-
.ok_or_else(|| bad_request("No jobs for pipeline"))?;
877+
.ok_or_else(|| bad_request("No jobs for pipeline"))?
878+
.id;
879879

880880
let mode = if req.force == Some(true) {
881881
RestartMode::force
882882
} else {
883883
RestartMode::safe
884884
};
885885

886+
// If user wants to ignore state, query max checkpoint epoch and compute threshold
886887
let ignore_before_epoch = if req.ignore_state.unwrap_or(false) {
887-
match config().job_controller {
888-
JobControllerMode::Controller => {
889-
// Controller mode uses this as an epoch threshold.
890-
api_queries::fetch_max_checkpoint_epoch(&db, &job.id, &auth_data.organization_id)
891-
.await?
892-
.into_iter()
893-
.next()
894-
.and_then(|r| r.max_epoch)
895-
.map(|max_epoch| max_epoch + 1)
896-
}
897-
JobControllerMode::Worker => {
898-
// Leader mode uses this as the generation that should start without state.
899-
Some(
900-
job.run_id
901-
.unwrap_or(0)
902-
.max(0)
903-
.checked_add(1)
904-
.and_then(|generation| generation.try_into().ok())
905-
.ok_or_else(|| bad_request("Job generation is too large to restart"))?,
906-
)
907-
}
908-
}
888+
api_queries::fetch_max_checkpoint_epoch(&db, &job_id, &auth_data.organization_id)
889+
.await?
890+
.into_iter()
891+
.next()
892+
.and_then(|r| r.max_epoch)
893+
.map(|max_epoch| max_epoch + 1)
909894
} else {
910895
None
911896
};
@@ -916,7 +901,7 @@ pub async fn restart_pipeline(
916901
&auth_data.user_id,
917902
&mode,
918903
&ignore_before_epoch,
919-
&job.id,
904+
&job_id,
920905
&auth_data.organization_id,
921906
)
922907
.await?;

crates/arroyo-controller/src/lib.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,6 @@ pub struct JobConfig {
111111
parallelism_overrides: HashMap<u32, usize>,
112112
restart_nonce: i32,
113113
restart_mode: RestartMode,
114-
/// Minimum checkpoint epoch in controller mode; generation to start without state in leader
115-
/// mode.
116114
ignore_state_before_epoch: Option<i32>,
117115
/// Per-job environment variables forwarded to workers at scheduling time.
118116
env_vars: serde_json::Value,

crates/arroyo-state-protocol/src/lib.rs

Lines changed: 0 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -759,7 +759,6 @@ mod tests {
759759
job_id: JobId::new("J"),
760760
generation: Generation(1),
761761
updated_at: from_micros(123),
762-
ignore_state: false,
763762
},
764763
false,
765764
)
@@ -815,7 +814,6 @@ mod tests {
815814
job_id: JobId::new("J"),
816815
generation: Generation(2),
817816
updated_at: from_micros(456),
818-
ignore_state: false,
819817
},
820818
false,
821819
)
@@ -847,62 +845,6 @@ mod tests {
847845
assert_eq!(written_manifest, expected_manifest);
848846
}
849847

850-
#[tokio::test]
851-
async fn initialize_generation_can_ignore_previous_checkpoint() {
852-
let store = MemoryProtocolStore::default();
853-
let paths = ProtocolPaths::new(PipelineId::new("P"), JobId::new("J"));
854-
write_current_generation(&store, &paths, Generation(2)).await;
855-
856-
let checkpoint_ref = paths.checkpoint_manifest(Generation(1), Epoch(1));
857-
let checkpoint = checkpoint_for_generation(Generation(1), 1, None, false);
858-
write_canonical_checkpoint(&store, &paths, &checkpoint_ref, &checkpoint).await;
859-
let previous_manifest =
860-
generation_manifest_for_generation(Generation(1), None, Some(checkpoint_ref));
861-
put_json(
862-
&store,
863-
&paths.generation_manifest(Generation(1)),
864-
&previous_manifest,
865-
)
866-
.await
867-
.unwrap();
868-
869-
let initialization = initialize_generation(
870-
&store,
871-
InitializeGenerationRequest {
872-
pipeline_id: PipelineId::new("P"),
873-
job_id: JobId::new("J"),
874-
generation: Generation(2),
875-
updated_at: from_micros(456),
876-
ignore_state: true,
877-
},
878-
false,
879-
)
880-
.await
881-
.unwrap();
882-
883-
let expected_manifest = GenerationManifest::new(
884-
PipelineId::new("P"),
885-
JobId::new("J"),
886-
Generation(2),
887-
None,
888-
456,
889-
);
890-
assert_eq!(
891-
initialization,
892-
GenerationInitialization::Initialized {
893-
generation_manifest: expected_manifest.clone(),
894-
recovery: GenerationRecovery::NoCheckpoint,
895-
}
896-
);
897-
898-
let written_manifest: GenerationManifest =
899-
read_json(&store, &paths.generation_manifest(Generation(2)))
900-
.await
901-
.unwrap()
902-
.expect("new generation manifest should be written");
903-
assert_eq!(written_manifest, expected_manifest);
904-
}
905-
906848
#[tokio::test]
907849
async fn initialize_generation_restores_previous_checkpoint_requiring_commit_replay() {
908850
let store = MemoryProtocolStore::default();
@@ -929,7 +871,6 @@ mod tests {
929871
job_id: JobId::new("J"),
930872
generation: Generation(2),
931873
updated_at: from_micros(456),
932-
ignore_state: false,
933874
},
934875
false,
935876
)
@@ -978,7 +919,6 @@ mod tests {
978919
job_id: JobId::new("J"),
979920
generation: Generation(3),
980921
updated_at: from_micros(789),
981-
ignore_state: false,
982922
},
983923
false,
984924
)
@@ -1026,7 +966,6 @@ mod tests {
1026966
job_id: JobId::new("J"),
1027967
generation: Generation(2),
1028968
updated_at: from_micros(456),
1029-
ignore_state: false,
1030969
},
1031970
false,
1032971
)
@@ -1083,7 +1022,6 @@ mod tests {
10831022
job_id: JobId::new("J"),
10841023
generation: Generation(3),
10851024
updated_at: from_micros(456),
1086-
ignore_state: false,
10871025
},
10881026
false,
10891027
)
@@ -1149,7 +1087,6 @@ mod tests {
11491087
job_id: JobId::new("J"),
11501088
generation: Generation(3),
11511089
updated_at: from_micros(456),
1152-
ignore_state: false,
11531090
},
11541091
false,
11551092
)
@@ -1187,7 +1124,6 @@ mod tests {
11871124
job_id: JobId::new("J"),
11881125
generation: Generation(2),
11891126
updated_at: from_micros(456),
1190-
ignore_state: false,
11911127
},
11921128
false,
11931129
)

crates/arroyo-state-protocol/src/workflow.rs

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,6 @@ pub struct InitializeGenerationRequest {
7171
pub job_id: JobId,
7272
pub generation: Generation,
7373
pub updated_at: SystemTime,
74-
/// Start this generation without restoring a checkpoint from an earlier generation.
75-
pub ignore_state: bool,
7674
}
7775

7876
/// Checkpoint, if any, that a newly initialized generation should restore from.
@@ -261,11 +259,7 @@ where
261259
});
262260
}
263261

264-
let recovery = if request.ignore_state {
265-
RecoverySearch::Found(GenerationRecovery::NoCheckpoint)
266-
} else {
267-
find_recovery_checkpoint(store, &paths, request.generation).await?
268-
};
262+
let recovery = find_recovery_checkpoint(store, &paths, request.generation).await?;
269263
let base_checkpoint_ref = match &recovery {
270264
RecoverySearch::Found(recovery) => match recovery {
271265
GenerationRecovery::NoCheckpoint => None,

crates/arroyo-worker/src/job_controller/controller.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -160,9 +160,6 @@ impl WorkerJobController {
160160
job_id: worker_context.job_id.clone(),
161161
generation: Generation(worker_context.generation),
162162
updated_at: SystemTime::now(),
163-
// The controller passes no parent checkpoint when this generation should start
164-
// without state (or when there is no state available).
165-
ignore_state: parent_ref.is_none(),
166163
},
167164
false,
168165
)

webui/src/routes/pipelines/PipelineDetails.tsx

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ export function PipelineDetails() {
138138

139139
async function updateJobState(stop: StopType) {
140140
console.log(`Setting pipeline stop_mode=${stop}`);
141-
await updatePipeline({ stop });
141+
updatePipeline({ stop });
142142
}
143143

144144
async function updateJobParallelism(parallelism: number) {
@@ -352,29 +352,23 @@ export function PipelineDetails() {
352352
let actionButton = <></>;
353353
if (pipeline) {
354354
editPipelineButton = <Button onClick={onConfigModalOpen}>Edit</Button>;
355-
const isStopped = job.state === 'Stopped' && pipeline.action != null;
356-
if (job.state === 'Failed' || isStopped) {
357-
const actionText = isStopped ? 'Start' : 'Restart';
355+
if (job.state == 'Failed') {
358356
actionButton = (
359357
<Popover trigger="hover" placement="bottom-start">
360358
<PopoverTrigger>
361359
<Button
362360
onClick={async () => {
363-
if (isStopped) {
364-
await updateJobState(pipeline.action!);
365-
} else {
366-
await restartPipeline(false);
367-
}
361+
await restartPipeline(false);
368362
}}
369363
>
370-
{actionText}
364+
Restart
371365
</Button>
372366
</PopoverTrigger>
373367
<PopoverContent width="auto">
374368
<PopoverArrow />
375369
<PopoverBody p={4}>
376370
<Button size="sm" colorScheme="red" onClick={onRestartWithoutStateModalOpen}>
377-
{actionText} Without State
371+
Restart Without State
378372
</Button>
379373
</PopoverBody>
380374
</PopoverContent>
@@ -395,10 +389,6 @@ export function PipelineDetails() {
395389
}
396390
}
397391

398-
const startingWithoutState = job.state === 'Stopped';
399-
const withoutStateAction = startingWithoutState ? 'Start' : 'Restart';
400-
const withoutStateActionPresentParticiple = startingWithoutState ? 'Starting' : 'Restarting';
401-
402392
const headerArea = (
403393
<Flex>
404394
<Box p={5}>
@@ -432,13 +422,13 @@ export function PipelineDetails() {
432422
<AlertDialogOverlay>
433423
<AlertDialogContent>
434424
<AlertDialogHeader fontSize="lg" fontWeight="bold">
435-
{withoutStateAction} Without State
425+
Restart Without State
436426
</AlertDialogHeader>
437427

438428
<AlertDialogBody>
439429
<Text>
440-
{withoutStateActionPresentParticiple} without state could lead to data loss,
441-
duplication, and incorrect results. Are you sure you want to continue?
430+
Restarting without state could lead to data loss, duplication, and incorrect
431+
results. Are you sure you want to continue?
442432
</Text>
443433
</AlertDialogBody>
444434

@@ -454,7 +444,7 @@ export function PipelineDetails() {
454444
}}
455445
ml={3}
456446
>
457-
{withoutStateAction} Without State
447+
Restart Without State
458448
</Button>
459449
</AlertDialogFooter>
460450
</AlertDialogContent>

0 commit comments

Comments
 (0)