Skip to content
This repository has been archived by the owner on Mar 31, 2023. It is now read-only.

Improve DPM and NCM performance #734

Merged
merged 76 commits into from
Feb 1, 2022
Merged

Conversation

DavidLiu506
Copy link
Contributor

@DavidLiu506 DavidLiu506 commented Jan 20, 2022

  1. In DPM, if neighbor state already pushed to NCM, don't push again.
  2. In NCM, query all neighbor state that ACA needed.
  3. Enable the partition awareness feature within scaling Kubernetes enviroment.
  4. Add service account that allows Alcor access Kubernetes API server.

DavidLiu506 and others added 30 commits June 29, 2021 12:07
@lgtm-com
Copy link

lgtm-com bot commented Jan 25, 2022

This pull request introduces 1 alert when merging 1bf4df8 into 4987793 - view on LGTM.com

new alerts:

  • 1 for Unused format argument

@lgtm-com
Copy link

lgtm-com bot commented Jan 25, 2022

This pull request introduces 1 alert when merging 3dbd205 into 4987793 - view on LGTM.com

new alerts:

  • 1 for Unused format argument

Copy link
Contributor

@xieus xieus left a comment

Choose a reason for hiding this comment

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

First batch of comments. Thank you @DavidLiu506

kubernetes/db/ignite/ignite_alcor.yaml Show resolved Hide resolved
kubernetes/db/ignite/ignite_config.xml Show resolved Hide resolved
kubernetes/services/route_manager.yaml Show resolved Hide resolved
<dependency>
<groupId>org.apache.ignite</groupId>
<artifactId>ignite-kubernetes</artifactId>
<version>2.12.0</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we upgrade our ignite-core dependency to 2.12 as well? The master stays on 2.10.

<dependency> <groupId>org.apache.ignite</groupId> <artifactId>ignite-core</artifactId> <version>2.10.0</version> </dependency>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good suggestions. will try it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In Medina, it used ignite lib8. it seems like 2.10 not works on kubernetes. @cj-chung

Copy link
Contributor

@xieus xieus left a comment

Choose a reason for hiding this comment

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

@DavidLiu506 Please take a look at the comment.

Comment on lines 64 to 66
@Autowired
private ResourceStateCache resourceStateCache;

Copy link
Contributor

@xieus xieus Jan 26, 2022

Choose a reason for hiding this comment

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

@DavidLiu506 I don't think gRPC client is a good place to read/write a cache, which is usually supposed to be done in upper service layer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good suggestions. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

We could move this post 1/30 release. Create an issue to track this enhancement item #735

kubernetes/services/dpm_manager.yaml Outdated Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Jan 26, 2022

This pull request introduces 1 alert when merging c0a19ee into 4987793 - view on LGTM.com

new alerts:

  • 1 for Unused format argument

@lgtm-com
Copy link

lgtm-com bot commented Jan 26, 2022

This pull request introduces 1 alert when merging 97e513a into 4987793 - view on LGTM.com

new alerts:

  • 1 for Unused format argument

@lgtm-com
Copy link

lgtm-com bot commented Jan 26, 2022

This pull request introduces 1 alert when merging a9d930e into 4987793 - view on LGTM.com

new alerts:

  • 1 for Unused format argument

@lgtm-com
Copy link

lgtm-com bot commented Jan 26, 2022

This pull request introduces 1 alert when merging 5b8dec8 into 58440cc - view on LGTM.com

new alerts:

  • 1 for Unused format argument

@lgtm-com
Copy link

lgtm-com bot commented Feb 1, 2022

This pull request introduces 1 alert when merging 91a3790 into 58440cc - view on LGTM.com

new alerts:

  • 1 for Unused format argument

@DavidLiu506
Copy link
Contributor Author

This PR works on Medina and Jenkins

@xieus xieus self-requested a review February 1, 2022 05:11
Copy link
Contributor

@xieus xieus left a comment

Choose a reason for hiding this comment

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

LGTM!

@xieus xieus merged commit acbea34 into futurewei-cloud:master Feb 1, 2022
@DavidLiu506 DavidLiu506 deleted the alcor/ncm branch May 28, 2022 01:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants