milenkovicm commented on code in PR #2269:
URL: 
https://github.com/apache/datafusion-ballista/pull/2269#discussion_r3752951641


##########
ballista/scheduler/src/scheduler_server/mod.rs:
##########
@@ -69,6 +67,48 @@ pub(crate) mod query_stage_scheduler;
 pub type SessionBuilder =
     Arc<dyn Fn(SessionConfig) -> datafusion::common::Result<SessionState> + 
Send + Sync>;
 
+/// Generates unique identifiers for submitted jobs.
+///
+/// Schedulers use one `JobIdGenerator` instance for their lifetime, calling
+/// [`next_id`](JobIdGenerator::next_id) once per job submission.
+/// Implementations must be safe to call concurrently from multiple tasks and
+/// must never return the same id twice, since job ids are used to key
+/// scheduler-wide state.
+///
+/// A default snowflake-based generator is used unless a
+/// custom implementation is supplied via
+/// 
[`SchedulerConfig::job_id_generator`](crate::config::SchedulerConfig::job_id_generator).
+#[async_trait::async_trait]
+pub trait JobIdGenerator: Sync + Send {
+    /// Returns a new, globally unique job id.
+    async fn next_id(&self) -> String;
+}
+
+/// A monotonically increasing sortable snowflake job id generator
+/// which does not capture machine id as part result.
+struct DefaultJobGenerator {
+    generator: AtomicSnowflakeGenerator<SnowflakeMastodonId, MonotonicClock>,
+}
+
+impl Default for DefaultJobGenerator {
+    fn default() -> Self {
+        let generator = AtomicSnowflakeGenerator::new(
+            0, // machine id is hard codded to 0

Review Comment:
   for that specific case we would need to use ULIDs or UUIDv7, if we keep 
snowflake style IDs we need to provide machine id somehow, which would probably 
mean users need to configure it manually or add some kind of infrastructure to 
provide machine id. 
   
   i suggest if we decide to go `ULID`s we can do it as a follow up, as my 
primary concern with this PR was sortable JIDs. wdyt @villebro 



##########
ballista/scheduler/src/scheduler_server/mod.rs:
##########
@@ -69,6 +67,48 @@ pub(crate) mod query_stage_scheduler;
 pub type SessionBuilder =
     Arc<dyn Fn(SessionConfig) -> datafusion::common::Result<SessionState> + 
Send + Sync>;
 
+/// Generates unique identifiers for submitted jobs.
+///
+/// Schedulers use one `JobIdGenerator` instance for their lifetime, calling
+/// [`next_id`](JobIdGenerator::next_id) once per job submission.
+/// Implementations must be safe to call concurrently from multiple tasks and
+/// must never return the same id twice, since job ids are used to key
+/// scheduler-wide state.
+///
+/// A default snowflake-based generator is used unless a
+/// custom implementation is supplied via
+/// 
[`SchedulerConfig::job_id_generator`](crate::config::SchedulerConfig::job_id_generator).
+#[async_trait::async_trait]
+pub trait JobIdGenerator: Sync + Send {
+    /// Returns a new, globally unique job id.
+    async fn next_id(&self) -> String;
+}
+
+/// A monotonically increasing sortable snowflake job id generator
+/// which does not capture machine id as part result.
+struct DefaultJobGenerator {
+    generator: AtomicSnowflakeGenerator<SnowflakeMastodonId, MonotonicClock>,
+}
+
+impl Default for DefaultJobGenerator {
+    fn default() -> Self {
+        let generator = AtomicSnowflakeGenerator::new(
+            0, // machine id is hard codded to 0

Review Comment:
   for that specific case we would need to use `ULID`s or `UUIDv7`, if we keep 
snowflake style IDs we need to provide machine id somehow, which would probably 
mean users need to configure it manually or add some kind of infrastructure to 
provide machine id. 
   
   i suggest if we decide to go `ULID`s we can do it as a follow up, as my 
primary concern with this PR was sortable JIDs. wdyt @villebro 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to