apache / apache/datafusion-ballista

NewType pattern for executor id's

Open
#1,876 1 comment 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
Rust
Stars
2.1k
Forks
320
Avg merge
1d 22h
Merged PRs (30d)
66

Description

**Is your feature request related to a problem or challenge? Please describe what you are trying to do.**
With the landing of #1852 , the next logical place to create a new type for is `ExecutorId`. This will help making clear when collections are keyed by executor id's or job id's, for example. The approach is similar (or, the same really) as for `JobId`.

**Describe the solution you'd like**
- Create a new type `ExecutorId(String)` (or `ExecutorId(Uuid)`)
- Replace all `executor_id` with this new type.

**Describe alternatives you've considered**
Executor ID's are UUIDs, and it was suggested to making it `ExecutorId(Uuid)` (see this [comment](https://github.com/apache/datafusion-ballista/pull/1852#issuecomment-4673754250)).

There are a few downsides to this that I could think of:
- We would have to change a lot of tests.
- Keeping `ExecutorId` semantically an opaque identifier (as it is now as well) feels more flexible.
- We wouldn't be able to reuse the introduced macro that automatically implements the ergonomic (imo, obviously biased as author) traits such as `From<&str>` and `Borrow`. The latter is important for easy use with `HashMap`s. I could of course make a new macro so this is not a very strong argument.

@martin-g : as the original commenter, what do you think? If you think `ExecutorId(Uuid)` makes more sense, then I will do that.

**Additional context**
This will await #1851 as that work is in all the same places.

Contributor guide

Open the contributing guide

Research direction

Start by reviewing the related JobId work in #1852 and the dependency in #1851, then locate the executor_id usages across the repository. Resolve whether ExecutorId should wrap String or Uuid, replace the relevant usages consistently, and update affected tests. Done means executor identifiers use the newtype without breaking collection lookups or existing behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
distributed-systems
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.