-
Notifications
You must be signed in to change notification settings - Fork 133
IGNITE-27437 Introduce PartitionCounterProvider #7324
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| * Parameters container for {@link PartitionCountProvider#calculate}. | ||
| */ | ||
| public final class PartitionCountCalculationParameters { | ||
| /** Data nodes filter. */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
such comments doesn't bring any value. Please provide meaningful one or just drop these
| * Test-only constructor. | ||
| */ | ||
| @TestOnly | ||
| public CatalogManagerImpl( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's have only one constructor. There are not so many usages
| newDefaultZoneId, | ||
| DEFAULT_ZONE_NAME, | ||
| DEFAULT_PARTITION_COUNT, | ||
| defaultZonePartitionCount(partitionCountProvider), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's better to set parameters explicitly
| /** | ||
| * Wrapper that allows to apply and use different partition count computing approaches. | ||
| */ | ||
| public final class PartitionCountProviderWrapper implements PartitionCountProvider { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrapper should be moved to runner module. Also, what about tests for this class?
| */ | ||
| public final class PartitionCountProviderWrapper implements PartitionCountProvider { | ||
| /** Wrapped delegate to compute partition count. */ | ||
| private PartitionCountProvider delegate; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's have it volatile
JIRA Ticket: IGNITE-27437
The goal
The goal is to introduce new functional interface that computes partition count for a new zone without partition count specified.
The reason
We want to implement dynamic default partition count computation in IGNITE-27279, this PR is prerequisite.
The solution
New entities are introduced. The wrapper will use later with new producer implementation. The intention of the wrapper is to decouple catalog manager and wrapped producer update, because the last may be done outside of
IgniteImplconstructor or require components that requireCatalogManageritself. Instead of new setter for the manager better to decouple it with wrapper.