AdheipSingh commented on PR #17:
URL: https://github.com/apache/druid-operator/pull/17#issuecomment-4374174970
#### **Note**: **The following is review is purely written by me and not
edited or reviewed or taken any inspiration by any AI.**
### Problem Statement
The issue raised is valid, as the operator reconciles multiple druid nodes
based on purely nodeType. The underlying structure is
```map[string]nodeSpec```, within nodeSpec the only way to segregate nodes is
based on nodeType. So if i have 10 historical with X,Y,Z names, they all will
get upgraded at same time, which is a pure disruption.
I would propose for an implementation which fixes the problem at its core,
and sticks to druid native design on how it represents nodeTypes and tiers.
### Approach
If i break down the main abstractions, we are dealing with 4 constructs. (
nodeType is present as of now, i plan to introduce order of upgrade, tiers and
order of upgrade of tiers ).
- OrderOfUpgrade
- OrderOfUpgradeOfTiers
- NodeTypes
- Tiers
As a user i should be able to define the following for my druid nodes.
- i want to upgrade my druid nodes in a certain order defined.
- i want to define tiers within my druid nodes.
- i want to define the order of upgrades of my tiers for each druid node
defined.
### Tiers
In druid we can categorize historicals as well as brokers into tiers. A tier
which defines separate groups of Historicals and Brokers to receive different
query assignments / loading rules etc.
How does this map in the operator spec ?
We introduce tier as a key within the nodeSpec, scoped same with nodeType.
```
historical-aws-az1:
tier: hot
nodeType: historical
historical-aws-az2:
tier: cold
nodeType:historical
broker-aws-az1:
tier: hot
nodeType: broker
broker-aws-az1:
tier: cold
nodeType: broker
.....
```
### OrderOfUpgrade
Remove the hardcoded order in the code, though the order is based on druid's
recommendation, a lot of times i had a custom order of upgrade. The main for
loop should construct an order based on this.
```
orderOfUpgrade:
- historical
- broker
- middlemanager
- coordinator
- router
- overlord
```
This is a []string structure and on each reconcile should be built.
### OrderOfUpgradeOfTiers
WIthin a nodeType we need to have the ability to define order of upgrade of
nodeTypes. So that needs to be defined as a separate structure for each tier
OrderOfUpgradeOfTiers:
```
- histroricals:
- hot
- cold
- glacier
- broker
- hot
- cold
```
Here's a combined spec.
```
spec:
orderOfUpgrade:
- historical
- broker
- middleManager
- coordinator
- router
- overlord
orderOfUpgradeOfTiers:
historical:
- hot
- cold
- glacier
broker:
- hot
- cold
nodes:
historical-aws-az1:
tier: hot
nodeType: historical
historical-aws-az2:
tier: cold
nodeType: historical
historical-aws-az3:
tier: glacier
nodeType: historical
broker-aws-az1:
tier: hot
nodeType: broker
broker-aws-az2:
tier: cold
nodeType: broker
middlemanagers:
nodeType: middleManager
coordinators:
nodeType: coordinator
routers:
nodeType: router
overlords:
nodeType: overlord
```
Upgrade execution:
1. historical:hot → historical-aws-az1
2. historical:cold → historical-aws-az2
3. historical:glacier → historical-aws-az3
4. broker:hot → broker-aws-az1
5. broker:cold → broker-aws-az2
6. middleManager → middlemanagers
7. coordinator → coordinators
8. router → routers
9. overlord → overlords
### Implementation
Signature remains as it is
```
func getNodeSpecsByOrder(m *v1alpha1.Druid) []*ServiceGroup {
// Returns ordered list of ServiceGroups ready for sequential rollout
}
```
The existing service group should be extended to use :
```
type ServiceGroup struct {
key string
nodeType string
tier string
spec v1alpha1.DruidNodeSpec
}
```
An ideal service group after construction should look like this:
```
[
{key: "historical-aws-az1", nodeType: "historical", tier: "hot", spec:
...},
{key: "historical-aws-az2", nodeType: "historical", tier: "cold", spec:
...},
{key: "historical-aws-az3", nodeType: "historical", tier: "glacier",
spec: ...},
{key: "broker-az1", nodeType: "broker", tier: "hot", spec: ...},
{key: "broker-az2", nodeType: "broker", tier: "cold", spec: ...},
{key: "mm-1", nodeType: "middleManager", tier: "", spec: ...},
]
```
This way we solve the problem at the core. I have half of the implementation
already done, and would like to raise a PR and get feedback. Also this is needs
to be backward compatible, so we make sure regardless of user specifying the
above we fallback to the default/current way.
--
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]
