HDDS-15409. SCM Pipeline Add SupportedStorageTier Field#10573
Conversation
|
@amaliujia @greenwich @chungen0126 @ivandika3 @peterxcli Please help to review |
chungen0126
left a comment
chungen0126
left a comment
There was a problem hiding this comment.
Thanks @xichen01 for working on this. Left some comments
| /** | ||
| * Set the storageTier supported by the pipeline. | ||
| */ | ||
| public void setSupportedStorageTier(List<StorageTier> supportedStorageTier) { |
There was a problem hiding this comment.
Using setSupportedStorageTier here breaks the immutable design of the Pipeline class. A better approach would be to follow the existing pattern of copyWithNodesInOrder to update the pipeline's state.
| } | ||
|
|
||
| private void updateSupportedStorageTier(DatanodeInfo datanodeInfo) { | ||
| if (scmContext.getScm() == null) { |
There was a problem hiding this comment.
Would it be better to use instanceof?
| if (scmContext.getScm() == null) { | |
| if (scmContext.getScm() instanceof StorageContainerManager) { |
| datanodeInfo.getID()); | ||
| } | ||
| } catch (PipelineNotFoundException e) { | ||
| LOG.debug("Reported Datanode {} pipeline {} is not found", |
There was a problem hiding this comment.
Would it be better to use the warn log level here?
| Pipeline pipeline = pipelineManager.getPipeline(pipelineId); | ||
| Set<StorageTier> currentTiers = | ||
| pipeline.getSupportedStorageTier() != null | ||
| ? new HashSet<>(pipeline.getSupportedStorageTier()) | ||
| : Collections.emptySet(); | ||
| List<StorageTier> newSupportedTiers = | ||
| NodeUtils.getDatanodesStorageTypes(pipeline.getNodes(), this); | ||
| Set<StorageTier> newSupportedTierSet = | ||
| new HashSet<>(newSupportedTiers); | ||
| if (!currentTiers.equals(newSupportedTierSet)) { | ||
| pipeline.setSupportedStorageTier(newSupportedTiers); | ||
| LOG.info("Updated supported storage tiers for Pipeline ID {} from {} " | ||
| + "to {} by Datanode {}", | ||
| pipeline.getId(), currentTiers, newSupportedTierSet, | ||
| datanodeInfo.getID()); | ||
| } |
There was a problem hiding this comment.
I'm not sure if we should update the pipeline when processing node report. My understanding is that pipeline updates should be scoped exclusively to PipelineReportHandler
2 participants
What changes were proposed in this pull request?
SCM Pipeline Add
supportedStorageTierFieldsupportedStorageTierfor Pipeline when the Pipeline be createdStorageTier, depending on the VolumeStorageTypeof the Datanodes that make up the Pipeline.StorageTypeVolumes, then the PipelinesupportedStorageTierisStorageTier.SSDandStorageTier.DISKsupportedStorageTierby the Pipeline is not fixed. This field will be set when the Pipeline is created and will be updated in thePipelineReportHandlerandNodeReportHandler.StorageTypeof the Volume of the Datanode in the Pipeline changes (for example, the Datanode configuration is manually modified), thesupportedStorageTierof the Pipeline will also changeFYI:
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-15409
How was this patch tested?
unit test