Skip to content

Conversation

@umamaheswararao
Copy link
Contributor

What changes were proposed in this pull request?

This patch implements the EC REconstruction coordinator functionality. With this patch, DN would be capable of reconstructing given missing indexes to given targets.

What is the link to the Apache JIRA

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

How was this patch tested?

Added tests.

@guihecheng
Copy link
Contributor

Hi Uma, by reading through the main flow, I think the implementation is good, I'll go through the details later, Thanks~

@guihecheng
Copy link
Contributor

Hi Uma, I've got 3 questions generally:

  1. We are going to treat the local target recovery just like remote target recovery, right?
  2. Do we plan to do cleanup on failures(e.g. writeChunk fails we may want to delete all recovered chunk files) in this patch or in a new one?
  3. Do we plan to do concurrent listBlock/CreateRecoveryingContainer/CloseContainer in this patch or in a new one?

@umamaheswararao
Copy link
Contributor Author

umamaheswararao commented Jun 14, 2022

HI @guihecheng thanks a lot for review and questions.
1 : Yes, for now we just treat everything as same. Local or remote. We can do local optimizations later.
2 : I think we can do that separately. Cleanup is not gauranteed always. So, we need some leasing kind of mechanism for avoiding collisions with new one. We can implement DN self cleaning after some long timeout.
3 : Yes, we will do that. In the initial cut, for stability purposes, we can just do sequential. When we do parallel, we need to consider other factors like CPU etc. May be we should do with small thread pool size. But I feel we will investigate that in next patches.
The main goal here is get the initial working version.

@guihecheng
Copy link
Contributor

HI @guihecheng thanks a lot for review and questions. 1 : Yes, for now we just treat everything as same. Local or remote. We can do local optimizations later. 2 : I think we can do that separately. Cleanup is not gauranteed always. So, we need some leasing kind of mechanism for avoiding collisions with new one. We can implement DN self cleaning after some long timeout. 3 : Yes, we will do that. In the initial cut, for stability purposes, we can just do sequential. When we do parallel, we need to consider other factors like CPU etc. May be we should do with small thread pool size. But I feel we will investigate that in next patches. The main goal here is get the initial working version.

Thanks for the replies, I agrees with the points.

@guihecheng
Copy link
Contributor

LGTM+1

Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

Thanks @umamaheswararao and @guihecheng for the patch. I have a few comments, mostly nits.

@adoroszlai
Copy link
Contributor

Thanks @umamaheswararao for updating the patch. There are two minor items left, but otherwise LGTM.

@umamaheswararao
Copy link
Contributor Author

Thanks @adoroszlai for taking a look. I have just corrected them. Thanks

@adoroszlai adoroszlai merged this pull request into apache:master Jun 16, 2022
@adoroszlai
Copy link
Contributor

I forgot to add @guihecheng as co-author in the commit message, sorry about that.

@umamaheswararao
Copy link
Contributor Author

we have just added:

Co-authored-by: Gui Hecheng <[email protected]>

Thanks a lot @guihecheng and @adoroszlai for reviews !!!
It's a good step forward for OfflineRecovery. Thanks for all the help from you.

umamaheswararao added a commit that referenced this pull request Jun 16, 2022
errose28 added a commit to errose28/ozone that referenced this pull request Jun 23, 2022
* master: (34 commits)
  HDDS-6868 Add S3Auth information to thread local (apache#3527)
  HDDS-6877. Keep replication port unchanged when restarting datanode in MiniOzoneCluster (apache#3510)
  HDDS-6907. OFS should create buckets with FILE_SYSTEM_OPTIMIZED layout. (apache#3528)
  HDDS-6875. Migrate parameterized tests in hdds-common to JUnit5 (apache#3513)
  HDDS-6924. OBJECT_STORE isn't flat namespaced (apache#3533)
  HDDS-6899. [EC] Remove warnings and errors from console during online reconstruction of data. (apache#3522)
  HDDS-6695. Enable SCM Ratis by default for new clusters only (apache#3499)
  HDDS-4123. Integrate OM Open Key Cleanup Service Into Existing Code (apache#3319)
  HDDS-6882. Correct exit code for invalid arguments passed to command-line tools. (apache#3517)
  HDDS-6890. EC: Fix potential wrong replica read with over-replicated container. (apache#3523)
  HDDS-6902. Duplicate mockito-core entries in pom.xml (apache#3525)
  HDDS-6752. Migrate tests with rules in hdds-server-scm to JUnit5 (apache#3442)
  HDDS-6806. EC: Implement the EC Reconstruction coordinator. (apache#3504)
  HDDS-6829. Limit the no of inflight replication tasks in SCM. (apache#3482)
  HDDS-6898. [SCM HA finalization] Modify acceptance test configuration to speed up test finalization (apache#3521)
  HDDS-6577. Configurations to reserve HDDS volume space. (apache#3484)
  HDDS-6870 Clean up isTenantAdmin to use UGI (apache#3503)
  HDDS-6872. TestAuthorizationV4QueryParser should pass offline (apache#3506)
  HDDS-6840. Add MetaData volume information to the SCM and OM - UI (apache#3488)
  HDDS-6697. EC: ReplicationManager - create class to detect EC container health issues (apache#3512)
  ...
guihecheng pushed a commit to guihecheng/ozone that referenced this pull request Jun 28, 2022
duongkame pushed a commit to duongkame/ozone that referenced this pull request Aug 16, 2022
HDDS-6806. EC: Implement the EC Reconstruction coordinator. (apache#3504)

Co-authored-by: Gui Hecheng <[email protected]>
(cherry picked from commit f57a019)
Change-Id: I77e71bbbc2286e699def332e2ae9d8c862df39d1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants