DAOS-4441 Placement: Modify Placement for OS Reintegration #2402

PetFet · 2020-04-11T00:16:28Z

Modified the placement API functionality so that now when
a reintegration operation is ongoing it will temporarily extend
the layout to include the reintegrating targets.

Signed-off-by: Peter Fetros [email protected]

Modified the placement API functionality so that now when a reintegration operation is ongoing it will temporarily extend the layout to include the reintegrating targets. Signed-off-by: Peter Fetros <[email protected]>

daosbuild1 · 2020-04-11T03:17:56Z

Test stage Functional completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-2402/1/execution/node/692/log

PetFet · 2020-04-13T22:58:22Z

#2284 Needs to be landed first if possible.

NiuYawei · 2020-04-20T07:15:50Z

src/placement/jump_map.c

+	PL_REBUILD,
+	PL_REINT,
+	PL_ADD,
+};


It's better to put some comments to explain these ops, that'll be helpful for understanding the code.

what is the difference between PL_PLACE_EXTENDED and PL_ADD?

Signed-off-by: Peter Fetros <[email protected]>

daosbuild1

Style warning(s) for job https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-2402/3/
Please review https://wiki.hpdd.intel.com/display/DC/Coding+Rules

daosbuild1 · 2020-04-20T18:45:25Z

src/placement/jump_map.c

+ * PL_PLACE_EXTENDED calculates an extended layout for use when there
+ * is a reintegration operation currently ongoing.
+ *
+ * PL_REINT calculates the post-reintegration layout for use during 


(style) trailing whitespace

daosbuild1 · 2020-04-20T18:45:26Z

src/placement/jump_map.c

@@ -31,6 +31,27 @@
 #include <daos/pool_map.h>
 #include <isa-l.h>

+
+/*
+ * These ops determine whether extra information is calculated during 


(style) trailing whitespace

daosbuild1 · 2020-04-20T18:45:37Z

Test stage checkpatch completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-2402/3/execution/node/60/log

daosbuild1 · 2020-04-20T18:47:00Z

Test stage Build RPM on CentOS 7 completed with status UNSTABLE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-2402/3/execution/node/209/log

Signed-off-by: Peter Fetros <[email protected]>

Updated patch

daosbuild1 · 2020-04-20T18:56:26Z

Test stage Build RPM on CentOS 7 completed with status UNSTABLE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-2402/4/execution/node/210/log

daosbuild1

Style warning(s) for job https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-2402/5/
Please review https://wiki.hpdd.intel.com/display/DC/Coding+Rules

src/placement/tests/place_obj_common.c

daosbuild1 · 2020-04-27T19:25:27Z

Test stage checkpatch completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-2402/5/execution/node/60/log

daosbuild1 · 2020-04-27T19:29:52Z

Test stage Build RPM on Leap 15 completed with status UNSTABLE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-2402/5/execution/node/206/log

daosbuild1 · 2020-04-27T19:31:22Z

Test stage Build RPM on CentOS 7 completed with status UNSTABLE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-2402/5/execution/node/209/log

daosbuild1 · 2020-04-27T19:39:25Z

Test stage Build on Ubuntu 18.04 with Clang completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-2402/5/execution/node/201/log

daosbuild1 · 2020-04-27T19:39:52Z

Test stage Build on Leap 15 with Intel-C and TARGET_PREFIX completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-2402/5/execution/node/202/log

daosbuild1 · 2020-04-27T19:43:15Z

Test stage Build on CentOS 7 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-2402/5/execution/node/259/log

Updated patch

daosbuild1 · 2020-04-27T22:25:55Z

Test stage Functional completed with status UNSTABLE. https://build.hpdd.intel.com/job/daos-stack/job/daos//view/change-requests/job/PR-2402/6/testReport/(root)/

daosbuild1 · 2020-06-04T01:20:58Z

Test stage checkpatch completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-2402/15/execution/node/57/log

Fixed several edge cases related to new extended layout sometimes returned during the test. - There was a problem with the new extended layout being returned during the IO test. This was because the IO test was triggering reintegration by making a call to pool_add_target which had not triggered reintegration in the past but was now causing a race between the rebuild process and the IO test. - It was also possible that while choosing the leader during placement, the newly extended shard would be chosen as the leader when it shouldn't have been. Now any shard which is not yet reintegrated will be skipped as the leader. Signed-off-by: Peter Fetros <[email protected]>

Updated patch

PetFet · 2020-06-04T22:01:33Z

OID Test Failure and Some Intermittent Rebuild Failures. These one's I do think are intermittent failures that are affecting everyone.

PetFet · 2020-06-05T18:59:48Z

Previously failed on rebuild Test 22, This can be found here https://jira.hpdd.intel.com/browse/DAOS-3952. otherwise no failures on run 18.

daosbuild1 · 2020-06-08T20:18:22Z

Test stage run_test.sh completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-2402/20/execution/node/389/log

NiuYawei · 2020-06-09T01:53:06Z

src/object/cli_obj.c

@@ -139,9 +139,6 @@ obj_shard_open(struct dc_object *obj, unsigned int shard, unsigned int map_ver,
 		D_GOTO(unlock, rc = -DER_NONEXIST);
 	}

-	/* XXX could be otherwise for some object classes? */
-	D_ASSERT(obj_shard->do_shard == shard);


In what use case they are not equal?

In this patches scenario.

When performing online features (reintegration/drain/etc.) we need to return an extended layout containing the shards we're moving data to, that way when we perform concurrent I/O the new writes also go to the targets we're reintegrating.

In this case we need a way keep track of which shards are in the extended layout by giving them the same shard number.

For example

[T1 T2 T3] <- original layout. [S1 S2 S3] <- shard ordering [T4 T2 T3] <- new layout after failure and rebuild. [S1 S2 S3] [T4 T2 T3 T1] <- for online reintegration, we need to write to everything because of concurrent I/O. [S1 S2 S3 S1] <- Reintegrating target being reintegrated for shard 1 [T1 T2 T3] <- reintegration complete. [S1 S2 S3]

NiuYawei · 2020-06-09T01:55:54Z

src/placement/tests/place_obj_common.c

 	for (i = 0; i < *spare_cnt; i++)
-		D_PRINT("shard %d, spare target rank %d\n",
-			shard_ids[i], spare_tgt_ranks[i]);



that'll leave an empty for loop?

That was a mistake, thanks for catching it!

Signed-off-by: Peter Fetros <[email protected]>

gnailzenh · 2020-06-11T04:59:30Z

src/placement/pl_map_common.c

+	int rc = 0;
+
+	grp_map = NULL;
+	grp_count = NULL;


could initialize when declaring them

gnailzenh · 2020-06-11T04:59:46Z

src/placement/pl_map_common.c

+		goto out;
+
+	D_ALLOC_ARRAY(grp_map, (layout->ol_nr / 8) + 1);
+	D_ALLOC_ARRAY(grp_count, layout->ol_grp_nr);


gnailzenh · 2020-06-11T05:00:32Z

src/placement/pl_map_common.c

+	grp_count = NULL;
+
+	/* Empty list, no extension needed */
+	if (extended_list == extended_list->next || layout->ol_grp_size == 1)


d_list_empty(extended_list)

gnailzenh · 2020-06-11T05:05:11Z

src/placement/pl_map_common.c

+	max_fail_grp = 0;
+
+	current = extended_list->next;
+	while (current != extended_list) {


d_list_for_each_safe?

gnailzenh · 2020-06-11T05:09:21Z

src/placement/pl_map_common.c

+			if (max_fail_grp < grp_count[grp])
+				max_fail_grp = grp_count[grp];
+		} else
+			d_list_del_init(&f_shard->fs_list);


style: use braces for all code blocks if any of code blocks of "if{}else" require braces

gnailzenh · 2020-06-11T05:09:39Z

src/placement/pl_map_common.c

+	new_group_size = layout->ol_grp_size + max_fail_grp;
+	D_ALLOC_ARRAY(new_shards, new_group_size * layout->ol_grp_nr);
+	if (new_shards == NULL)
+		return -DER_NOMEM;


gnailzenh · 2020-06-11T05:13:11Z

src/placement/pl_map_common.c

+	if (extended_list == extended_list->next || layout->ol_grp_size == 1)
+		goto out;
+
+	D_ALLOC_ARRAY(grp_map, (layout->ol_nr / 8) + 1);


no big deal, but we normally use this: (layout->ol_nr + 1)/8, otherwise you got two bytes if nr==8.

gnailzenh · 2020-06-11T05:18:18Z

src/placement/pl_map_common.c


+	current = extended_list->next;
+	while (current != extended_list) {


d_list_for_each_entry?

gnailzenh · 2020-06-11T06:20:21Z

src/placement/pl_map_common.c

+	if (new_shards == NULL)
+		return -DER_NOMEM;
+
+	while (k < layout->ol_nr) {


easier to understand in this way? " for (i = k = 0; k < layout->ol_nr;)"
if we need to set initial value of loop, I think it's cleaner to just use "for" statement

daosbuild1 · 2020-06-11T17:00:08Z

Test stage Functional completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-2402/22/display/redirect

daosbuild1 · 2020-06-11T17:03:36Z

Test stage Functional_Hardware_Large completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-2402/22/display/redirect

daosbuild1 · 2020-06-11T19:11:33Z

Test stage Functional completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-2402/23/display/redirect

daosbuild1 · 2020-06-11T19:16:35Z

Test stage Functional_Hardware_Medium completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-2402/23/display/redirect

PetFet · 2020-06-16T19:22:31Z

Currently failing Rebuild Test #3.
The client log reports this error:

06/15-22:21:58.06 wolf-52 DAOS[32257/32257] object INFO src/object/obj_verify.c:539 dc_obj_verify_cmp() 1152921590623633466.31 (reps 3, inconsistent) shard 0 has rec type 0, but shard 1 has rec type 4.

06/15-22:21:58.06 wolf-52 DAOS[32257/32257] object ERR  src/object/obj_verify.c:710 dc_obj_verify_rdg() Failed to verify cmp: DER_MISMATCH(-1031)

Di is working on a patch for this issue (DAOS-4623)

daosbuild1 · 2020-06-16T21:10:28Z

Test stage Functional_Hardware_Large completed with status UNSTABLE. https://build.hpdd.intel.com/job/daos-stack/job/daos//view/change-requests/job/PR-2402/26/testReport/(root)/

* DAOS-4441 Placement: Modify Placement for OS Reintegration Modified the placement API functionality so that now when a reintegration operation is ongoing it will temporarily extend the layout to include the reintegrating targets. Fixed several edge cases related to new extended layout sometimes returned during the test. - There was a problem with the new extended layout being returned during the IO test. This was because the IO test was triggering reintegration by making a call to pool_add_target which had not triggered reintegration in the past but was now causing a race between the rebuild process and the IO test. - It was also possible that while choosing the leader during placement, the newly extended shard would be chosen as the leader when it shouldn't have been. Now any shard which is not yet reintegrated will be skipped as the leader. Signed-off-by: Peter Fetros <[email protected]>

DAOS-4441 Placement: Modify Placement for OS Reintegration

c1592ee

Modified the placement API functionality so that now when a reintegration operation is ongoing it will temporarily extend the layout to include the reintegrating targets. Signed-off-by: Peter Fetros <[email protected]>

Merge remote-tracking branch 'origin/master' into DAOS-4441

697e44a

PetFet requested review from NiuYawei and gnailzenh April 13, 2020 22:56

PetFet requested review from byron-marohn and wangdi1 April 13, 2020 22:58

NiuYawei reviewed Apr 20, 2020

View reviewed changes

PetFet added 2 commits April 20, 2020 18:29

Merge remote-tracking branch 'origin/master' into DAOS-4441

39370a5

Added Comments Explaining What the Current Ops Do.

b334eab

Signed-off-by: Peter Fetros <[email protected]>

daosbuild1 previously requested changes Apr 20, 2020

View reviewed changes

Style Fixes

1919e92

Signed-off-by: Peter Fetros <[email protected]>

Merge remote-tracking branch 'origin/master' into DAOS-4441

1f1edc7

NiuYawei previously approved these changes Apr 23, 2020

View reviewed changes

PetFet dismissed NiuYawei’s stale review via 2a14396 April 27, 2020 19:11

daosbuild1 previously requested changes Apr 27, 2020

View reviewed changes

src/placement/tests/place_obj_common.c Outdated Show resolved Hide resolved

src/placement/tests/place_obj_common.c Outdated Show resolved Hide resolved

PetFet force-pushed the DAOS-4441 branch from 2a14396 to 1f1edc7 Compare April 27, 2020 20:33

PetFet force-pushed the DAOS-4441 branch from 1c8b609 to de9fc85 Compare June 4, 2020 01:22

Merge remote-tracking branch 'origin/master' into DAOS-4441

9eeb211

Merge remote-tracking branch 'origin/master' into DAOS-4441

7479f24

NiuYawei reviewed Jun 9, 2020

View reviewed changes

PetFet added 2 commits June 9, 2020 18:24

Merge remote-tracking branch 'origin/master' into DAOS-4441

e26270a

Fixed accidental removal of printed statements.

f995c80

Signed-off-by: Peter Fetros <[email protected]>

PetFet requested a review from NiuYawei June 9, 2020 21:55

NiuYawei approved these changes Jun 10, 2020

View reviewed changes

gnailzenh approved these changes Jun 11, 2020

View reviewed changes

Merge remote-tracking branch 'origin/master' into DAOS-4441

381204b

PetFet added 3 commits June 12, 2020 18:50

Merge remote-tracking branch 'origin/master' into DAOS-4441

8ffabac

Merge remote-tracking branch 'origin/master' into DAOS-4441

8b983d1

Merge remote-tracking branch 'origin/master' into DAOS-4441

810f09f

gnailzenh approved these changes Jun 17, 2020

View reviewed changes

gnailzenh merged commit 28611b4 into master Jun 17, 2020

dmiter deleted the DAOS-4441 branch November 3, 2020 21:56


		current = extended_list->next;
		while (current != extended_list) {

DAOS-4441 Placement: Modify Placement for OS Reintegration #2402

DAOS-4441 Placement: Modify Placement for OS Reintegration #2402

Conversation

PetFet commented Apr 11, 2020

daosbuild1 commented Apr 11, 2020

PetFet commented Apr 13, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daosbuild1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daosbuild1 commented Apr 20, 2020

daosbuild1 commented Apr 20, 2020

daosbuild1 commented Apr 20, 2020

daosbuild1 left a comment

Choose a reason for hiding this comment

daosbuild1 commented Apr 27, 2020

daosbuild1 commented Apr 27, 2020

daosbuild1 commented Apr 27, 2020

daosbuild1 commented Apr 27, 2020

daosbuild1 commented Apr 27, 2020

daosbuild1 commented Apr 27, 2020

daosbuild1 commented Apr 27, 2020

daosbuild1 commented Jun 4, 2020

PetFet commented Jun 4, 2020

PetFet commented Jun 5, 2020 • edited Loading

daosbuild1 commented Jun 8, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daosbuild1 commented Jun 11, 2020

daosbuild1 commented Jun 11, 2020

daosbuild1 commented Jun 11, 2020

daosbuild1 commented Jun 11, 2020

PetFet commented Jun 16, 2020 • edited Loading

daosbuild1 commented Jun 16, 2020

PetFet commented Jun 5, 2020 •

edited

Loading

PetFet commented Jun 16, 2020 •

edited

Loading