Skip to content
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

HDDS-5670. ContainerBalancer should get OzoneConfiguration from ContainerBalancerConfiguration. #2577

Merged
merged 2 commits into from Sep 9, 2021

Conversation

siddhantsangwan
Copy link
Contributor

@siddhantsangwan siddhantsangwan commented Aug 25, 2021

What changes were proposed in this pull request?

Currently, ContainerBalancer creates a new OzoneConfiguration in ContainerBalancer#start and uses that. Instead, it should get the OzoneConfiguration object from ContainerBalancerConfiguration.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-5670

How was this patch tested?

TestContainerBalancer UT

@siddhantsangwan
Copy link
Contributor Author

@lokeshj1703 @JacksonYao287 please take a look.

Copy link
Contributor

@JacksonYao287 JacksonYao287 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @siddhantsangwan for this work, the changes looks good ! i have a minor comment inline, please take a look

@@ -111,10 +111,14 @@
*/
public ContainerBalancerConfiguration(
OzoneConfiguration ozoneConfiguration) {
this.ozoneConfiguration = ozoneConfiguration;
if (ozoneConfiguration == null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually, ozoneConfiguration should not be null. we'd better to use Preconditions.checkNotNull instead here.

@siddhantsangwan
Copy link
Contributor Author

@JacksonYao287 thanks for the review! I've updated according to your comment.

Copy link
Contributor

@JacksonYao287 JacksonYao287 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM +1! thanks @siddhantsangwan for the work

@lokeshj1703 lokeshj1703 changed the title HDDS-5670. ContainerBalancer should get OzoneConfiguration from Conta… HDDS-5670. ContainerBalancer should get OzoneConfiguration from ContainerBalancerConfiguration. Sep 9, 2021
@lokeshj1703 lokeshj1703 merged commit e9cce6e into apache:master Sep 9, 2021
@lokeshj1703
Copy link
Contributor

@siddhantsangwan Thanks for the contribution! @JacksonYao287 Thanks for the review! I have committed the PR to master branch.

errose28 added a commit to errose28/ozone that referenced this pull request Sep 10, 2021
* master: (21 commits)
  HDDS-5502. [OFS] URI parser throws URISyntaxException when path contains space (apache#2500)
  HDDS-5715. Make XceiverServerRatis#raftGids a thread-safe set. (apache#2613)
  HDDS-5699. Added Log to show why a container was marked UNHEALTHY. (apache#2627)
  HDDS-5723. Increase time limit of Ozone acceptance tests. (apache#2620)
  HDDS-5718. Refactor TestXceiverClientManager to reuse mini-clusters (apache#2616)
  HDDS-5724. Add RaftpeerId when getting scm roles (apache#2622)
  HDDS-5711. support -1 for running balancer infinitely (apache#2621)
  HDDS-5670. ContainerBalancer should get OzoneConfiguration from ContainerBalancerConfiguration. (apache#2577)
  HDDS-5638. Fix docker-compose to make Recon come up. (apache#2563)
  HDDS-5726. Skip remove for already removed pipeline. (apache#2624)
  HDDS-5719. Reduce number of mini-clusters needed for decommission tests (apache#2617)
  HDDS-5716. Fix create key failure error log print (apache#2614)
  HDDS-5678. Handle unsecure SCM HA converted to secure SCM HA. (apache#2596)
  HDDS-5432. Enable downgrade testing after 1.1.0 release. (apache#2484)
  HDDS-5709. do not call removeTransactionsFromDB if nothing to remove (apache#2608)
  HDDS-5700. Improve LOG message of decommission progress. (apache#2598)
  HDDS-5690. Speed up TestContainerReplication by removing testSkipDemmissionAndMaintenanceNode (apache#2591)
  HDDS-5706. Fix ReplicationManager zero metrics for inflight actions. (apache#2605)
  HDDS-5667. documentation page layout (apache#2604)
  HDDS-5644. Speed up decommission tests using a background Mini Cluster provider (apache#2554)
  ...
@siddhantsangwan siddhantsangwan deleted the HDDS-5670 branch September 16, 2021 11:15
sadanand48 pushed a commit that referenced this pull request Oct 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants