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

DAOS-4763 dfs: New API for anchor split to allow parallelizing dfs_readdir #2652

Merged
merged 5 commits into from
May 21, 2020

Conversation

mchaarawi
Copy link
Contributor

@mchaarawi mchaarawi commented May 7, 2020

To optimize find performance, we need to be able to execute a parallel
readdir. To do that we need to be able to split an anchor among multiple
clients.

This PR adds 2 new API calls each to DAOS obj and DFS:

  1. daos/dfs_obj_anchor_split() to suggest a splitting of anchors by DAOS (internally based on number of shards), but also let user select a desired split factor. The latter is not supported yet.
  2. daos/dfs_obj_anchor_set() to set 1 anchor based on an index from the num of anchors returned from dfs_obj_anchor_split().

Signed-off-by: Mohamad Chaarawi mohamad.chaarawi@intel.com

To optimize find performance, we need to be able to execute a parallel
readdir. To do that we need to be able to split an anchor among multiple
clients. For this, we will split it depending on the number of shards a
directory is shared over.

We need a new API for the anchor split based on the shard number, and a
new API for DFS to return the number of shards of a directory or file.

Signed-off-by: Mohamad Chaarawi <mohamad.chaarawi@intel.com>
@mchaarawi mchaarawi force-pushed the mschaara/split_anchor branch from 7995d93 to a0b33e5 Compare May 7, 2020 20:15
@mchaarawi mchaarawi changed the title WIP - split anchor for parallel readdir DAOS-4763 dfs + object: Add new API for anchor split and DFS get_shard. May 7, 2020
@mchaarawi mchaarawi requested a review from johannlombardi May 8, 2020 14:47
*num_shards = layout->ol_nr;
daos_obj_layout_free(layout);
return 0;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not clear on how this is intended to be used for an array TBH. The number of shards in the future might not be stable too (with GIGA+), so i am not sure whether this is a good idea to expose it to the end users.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yea i was debating this myself.. I'll remove that.

/**
* Set an anchor for enumeration for one shard only.
*
* \param[in] oh Object open handle.
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no object handle in the API below.

* \param[in/out]
* anchor Anchor modified for 1 shard only
*
* \return 0 Success and consistent
Copy link
Contributor

Choose a reason for hiding this comment

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

???

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it should always succeed.. i don't know where i copied that from

* \return 0 Success and consistent
*/
int
daos_obj_anchor_split(uint32_t shard, daos_anchor_t *anchor);
Copy link
Contributor

Choose a reason for hiding this comment

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

What I had in mind was more something like this:

/**
  * Generate an array of anchors that split the dkey address space for parallel enumeration.
  *
  * \param[in]	oh	Object open handle.
  * \param[in/out]
  *                    nr	Size of anchors array. If 0 is passed, return the optimal size back.
  *                    	 Number of split anchors returned. 
  * \param[in/out]
  *		 anchors	Array of split anchors
  *
  * \return ...
  */
 int
 daos_obj_anchor_split(daos_handle_t oh, uint32_t *nr, daos_anchor_t *anchors);

Copy link
Contributor Author

@mchaarawi mchaarawi May 9, 2020

Choose a reason for hiding this comment

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

hmm, but how would you distribute the array of anchors to everyone?
The way it works today is that one guy get the optimal number of anchors (which is number of shards today), and inserts works into the queue for each shard. Others pick that up and generate an anchor using this function for a particular shard. otherwise I will probably need to modify this patch more to allow an anchor to span a range but not all shards, which isn't supported today.

dc_obj_anchor_split(uint32_t shard, daos_anchor_t *anchor) {
daos_anchor_set_zero(anchor);
dc_obj_shard2anchor(anchor, shard);
daos_anchor_set_flags(anchor, DIOF_TO_SPEC_SHARD);
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it return DAOS_ANCHOR_TYPE_EOF when we reach the end of the shard?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, but it required that fix i made above on line 2156

Signed-off-by: Mohamad Chaarawi <mohamad.chaarawi@intel.com>
@mchaarawi mchaarawi changed the title DAOS-4763 dfs + object: Add new API for anchor split and DFS get_shard. DAOS-4763 dfs: Add new API for anchor split to allow parallelizing dfs_readdir May 12, 2020
@mchaarawi mchaarawi changed the title DAOS-4763 dfs: Add new API for anchor split to allow parallelizing dfs_readdir DAOS-4763 dfs: New API for anchor split to allow parallelizing dfs_readdir May 12, 2020
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

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

/**
* Set an anchor with an index based on split done with dfs_obj_anchor_split.
* The anchor passed will be re-intialized and set to start and finish iteration
* based on the specified index.
Copy link
Collaborator

Choose a reason for hiding this comment

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

(style) trailing whitespace

*/
int
dfs_obj_anchor_set(dfs_obj_t *obj, uint32_t index, daos_anchor_t *anchor);

Copy link
Collaborator

Choose a reason for hiding this comment

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

(style) trailing whitespace

/** TBD - support more than per shard iteration */
if (*nr !=0 && *nr != layout->ol_nr) {
D_ERROR("For now, num anchors should be the same as what is"
"reported as optimal\n");
Copy link
Collaborator

Choose a reason for hiding this comment

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

(style) break quoted strings at a space character

if (*nr !=0 && *nr != layout->ol_nr) {
D_ERROR("For now, num anchors should be the same as what is"
"reported as optimal\n");
D_GOTO(out, rc = ENOSYS);
Copy link
Collaborator

Choose a reason for hiding this comment

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

(style) ENOSYS means 'invalid syscall nr' and nothing else

return rc;

/** TBD - support more than per shard iteration */
if (*nr !=0 && *nr != layout->ol_nr) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

(style) spaces required around that '!=' (ctx:WxV)

@daosbuild1
Copy link
Collaborator

@daosbuild1
Copy link
Collaborator

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

Signed-off-by: Mohamad Chaarawi <mohamad.chaarawi@intel.com>
@daosbuild1 daosbuild1 dismissed their stale review May 13, 2020 21:10

Updated patch

@mchaarawi mchaarawi requested a review from johannlombardi May 18, 2020 13:36
done  for regular KV object.

Signed-off-by: Mohamad Chaarawi <mohamad.chaarawi@intel.com>
@daosbuild1
Copy link
Collaborator

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

@daosbuild1
Copy link
Collaborator

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

@daosbuild1
Copy link
Collaborator

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

@mchaarawi mchaarawi requested review from wangdi1 and gnailzenh May 19, 2020 13:55
@mchaarawi
Copy link
Contributor Author

@wangdi1 i think at least one of the rebuild test failures might be due to my change to fix iteration to stop when the SPEC flag is added to the anchor.
Could you please advise?

@wangdi1
Copy link
Contributor

wangdi1 commented May 19, 2020

@wangdi1 i think at least one of the rebuild test failures might be due to my change to fix iteration to stop when the SPEC flag is added to the anchor.
Could you please advise?

@mchaarawi I checked the result for #6, it looks like
The failure in rebuild0-10 caused by DAOS-4623
The failure in rebuild26 caused by DAOS-4626,

I do not see other rebuild failures. or you mean other failures?

@mchaarawi
Copy link
Contributor Author

@wangdi1 i think at least one of the rebuild test failures might be due to my change to fix iteration to stop when the SPEC flag is added to the anchor.
Could you please advise?

@mchaarawi I checked the result for #6, it looks like
The failure in rebuild0-10 caused by DAOS-4623
The failure in rebuild26 caused by DAOS-4626,

I do not see other rebuild failures. or you mean other failures?

well in here:
https://build.hpdd.intel.com/blue/organizations/jenkins/daos-stack%2Fdaos/detail/PR-2652/6/tests

I see 1 new failures and three existing. i thought the 1 new is caused by my patch.
if not, and it is known, then please never-mind this.

@wangdi1
Copy link
Contributor

wangdi1 commented May 19, 2020

@wangdi1 i think at least one of the rebuild test failures might be due to my change to fix iteration to stop when the SPEC flag is added to the anchor.
Could you please advise?
@mchaarawi I checked the result for #6, it looks like
The failure in rebuild0-10 caused by DAOS-4623
The failure in rebuild26 caused by DAOS-4626,
I do not see other rebuild failures. or you mean other failures?

well in here:
https://build.hpdd.intel.com/blue/organizations/jenkins/daos-stack%2Fdaos/detail/PR-2652/6/tests

I see 1 new failures and three existing. i thought the 1 new is caused by my patch.
if not, and it is known, then please never-mind this.

oh, that is 4623. Hmm, not sure why this is marked as new.

@johannlombardi johannlombardi merged commit ea8e4f0 into master May 21, 2020
@johannlombardi johannlombardi deleted the mschaara/split_anchor branch May 21, 2020 06:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants