Skip to content

Client API implementation #622

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.

Sign up for GitHub

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

Jump to bottom
Merged
adammoody merged 1 commit into from
May 17, 2021
Merged

Client API implementation #622

adammoody merged 1 commit into from
May 17, 2021

Conversation

@MichaelBrim
Copy link
Collaborator

@MichaelBrim MichaelBrim commented Apr 22, 2021 •
edited
Loading

Description

Introduces the client API (client/src/unifyfs_api.h) and associated library (libunifyfs_api), along with a set of unit tests.

Motivation and Context

The client API is intended for use by I/O middleware libraries or applications that want to bypass the typical POSIX I/O layer.

How Has This Been Tested?

Tested in Docker and on OLCF Summit.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Testing (addition of new tests or update to current tests)
  • Documentation (a change to man pages or other documentation)

Checklist:

  • My code follows the UnifyFS code style requirements.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • All commit messages are properly formatted.

@MichaelBrim MichaelBrim removed the WIP label Apr 27, 2021
@MichaelBrim MichaelBrim marked this pull request as ready for review April 27, 2021 18:14
adammoody
adammoody approved these changes May 5, 2021
View reviewed changes
Copy link
Collaborator

@adammoody adammoody left a comment

Choose a reason for hiding this comment

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

Thanks @MichaelBrim ! This is great work, with some really nice clean up too. I haven't gone through it in full detail yet, but it all looks solid to me in a quick review. I'm fine with merging.

CamStan
CamStan approved these changes May 6, 2021
View reviewed changes
Copy link
Member

@CamStan CamStan left a comment

Choose a reason for hiding this comment

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

Docs? There, I said it! 😆

This is great @MichaelBrim! 🥳

@MichaelBrim
Copy link
Collaborator Author

MichaelBrim commented May 7, 2021

I realized that the alpha implementation was missing support for the transfer methods, so I added that. Now, similar to what Jenna and Craig have been seeing, the stage-transfer unit test (0700) is failing in Travis. I can reproduce the failure in my Docker environment, and am currently debugging.

@MichaelBrim MichaelBrim force-pushed the client-api-impl branch 6 times, most recently from 369e360 to 71ffffa Compare May 7, 2021 20:25
@MichaelBrim
Copy link
Collaborator Author

MichaelBrim commented May 7, 2021

Well, I resolved the stage-out issue I could reproduce in my docker environment (and fixed a couple other random things), but the final file comparison check for 0700 is still failing in Travis, so I restored the test_might_fail.

CamStan
CamStan requested changes May 10, 2021
View reviewed changes
Copy link
Member

@CamStan CamStan left a comment

Choose a reason for hiding this comment

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

Mind making one more adjustment to the stage tests?

@MichaelBrim
Copy link
Collaborator Author

MichaelBrim commented May 10, 2021

@CamStan yes, I'll add those changes

@MichaelBrim
Copy link
Collaborator Author

MichaelBrim commented May 10, 2021

@CamStan those changes seem to have made Travis happy :)

@MichaelBrim MichaelBrim force-pushed the client-api-impl branch 6 times, most recently from 4a35da4 to 135552f Compare May 12, 2021 19:03
@MichaelBrim
Copy link
Collaborator Author

MichaelBrim commented May 12, 2021

I have added a read-the-docs page on using the client API. Feel free to re-review or just go ahead and merge.

@MichaelBrim
initial client API library implementation
95f7f72 
* implements the alpha version of the client API and library.
* includes new unit tests to exercise the library.

Also includes:
* fix for missing req completion notice for hole extent
* fix for write after laminate
* eliminate shadow copy of file attributes in client metadata
* allow symbolic file access permissions in checkpatch
* fix unifyfs-stage unit tests
* fix to clean up logio shmem regions on server exit

TEST_CHECKPATCH_SKIP_FILES="common/src/unifyfs_configurator.c"
TEST_CHECKPATCH_SKIP_FILES+=",t/0700-unifyfs-stage-full.t"
TEST_CHECKPATCH_SKIP_FILES+=",t/9300-unifyfs-stage-isolated.t"
@adammoody adammoody merged commit 615122a into llnl:dev May 17, 2021
@MichaelBrim MichaelBrim deleted the client-api-impl branch May 24, 2021 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adammoody adammoody adammoody approved these changes

@CamStan CamStan CamStan approved these changes

+1 more reviewer

@craigsteffen craigsteffen craigsteffen left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

enhancement ready to merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@MichaelBrim @adammoody @craigsteffen @CamStan