CORTX-33972: DI : 43motr-sync-replication ST, be-ut, io-nw-xfer-ut fix #2100
Conversation
*43motr-sync-replication *be-ut failure in emap *io-nw-xfer-ut failure in target_ioreq_seg_add Signed-off-by: Rajat Patil <rajat.r.patil@seagate.com> Co-authored-by: Vidyadhar Pinglikar <vidyadhar.pinglikar@seagate.com>
rajatpatil98
requested review from
mehjoshi,
madhavemuri,
nikitadanilov,
yeshpal-jain-seagate,
huanghua78,
andriytk,
siningwuseagate,
vidyadhar-pinglikar,
shashank-parulekar,
nkommuri,
sg-shankar,
yatin-mahajan and
rkothiya
as code owners
|
Thanks for your contribution! |
|
I have reviewed this patch. I don't have objections. But this needs approval from someone who is familiar with DI. |
|
@vidyadhar-pinglikar , @huanghua78 , @rajatpatil98 (N < K) is only valid for replication. Are we sure these changes are for replication and not for parity calculation ? I could not follow the calculation of goff for replication. |
@shashank-parulekar . these changes are just for replication where N=1, K>N; also the offset changes (which are to ti_goff_ivec) here is not for the actual goff put on the disc for parity; but are used specifically to find the right map-idx, n unit idx offset during checksum computation and does not deal with actual goff used for writing data; |
|
Thanks for your contribution! |
| @@ -975,10 +991,24 @@ static void target_ioreq_calc_idx(struct m0_op_io *ioo, | |||
| &fop_cs_data->cd_idx[fop_cs_data->cd_num_units]; | |||
| M0_ASSERT(cs_idx->ci_pg_idx == UINT32_MAX && | |||
There was a problem hiding this comment.
I have a question here:
ci_pg_idx is declared as uint64_t.
Why are we comparing it against UINT32_MAX, but not UINT64_MAX?
There was a problem hiding this comment.
in ioreq_fop_checksum_data_init, it looks to have initialized with UINT32_MAX instead of UINT64_MAX; will correct it during next work around DI
| if ((unit_type == M0_PUT_PARITY) && | ||
| (layout_k(play) > layout_n(play))) { | ||
| m0_bcount_t goff_delta = (layout_k(play) - | ||
| layout_n(play)) * | ||
| gob_offset; | ||
| goff_cksum += goff_delta; |
There was a problem hiding this comment.
I didn't see this algorithm in the doc:
https://seagate-systems.atlassian.net/wiki/spaces/PRIVATECOR/pages/1010663429/Motr+DI?src=mail&src.mail.action=view&src.mail.notification=com.atlassian.confluence.plugins.confluence-notifications-batch-plugin%3Abatching-notification&src.mail.recipient=8a7f808a7812f8e60178188e03000549&src.mail.timestamp=1652423407033
Maybe we need to add a paragraph there to explain such special case.
There was a problem hiding this comment.
sure, this particular case was a miss and was captured little late during check in time; RMW ST with configuration of sync replication didnot capture this which was tested heavily for DI; thats one part to additionally check in that rmw ST :)
Problem: 43motr-sync-replication ST fails. be-ut failure in emap and io-nw-xfer-ut failure in target_ioreq_seg_add fails. Solution: 43motr-sync-replication fixed for N < K by shifting goff by (K-N)*offset. be-ut failure in emap is fixed by changing EXTMAP_UT_UNIT_SIZE to 16 from 10 as it should be in range of power 2. io-nw-xfer-ut is fixed by allocating ti_goff_ivec. Signed-off-by: Rajat Patil <rajat.r.patil@seagate.com> Co-authored-by: Vidyadhar Pinglikar <vidyadhar.pinglikar@seagate.com>
*43motr-sync-replication
*be-ut failure in emap
*io-nw-xfer-ut failure in target_ioreq_seg_add
Signed-off-by: Rajat Patil rajat.r.patil@seagate.com
Co-authored-by: Vidyadhar Pinglikar vidyadhar.pinglikar@seagate.com
Problem Statement
Design
Coding
Checklist for Author
Testing
Checklist for Author
Impact Analysis
Checklist for Author/Reviewer/GateKeeper
Review Checklist
Checklist for Author
Documentation
Checklist for Author