Skip to content

Conversation

@a1exsh
Copy link

@a1exsh a1exsh commented Sep 10, 2015

  • directio
  • blocksize
  • sequential access

benschweizer pushed a commit that referenced this pull request Sep 10, 2015
added direct io, sequential access pattern and initial blocksize option
@benschweizer benschweizer merged commit 7d40bc9 into benschweizer:master Sep 10, 2015
@benschweizer
Copy link
Owner

Hi, direct io mode should be optional it should use os.open and not a 3rd party module; not using direct io is intended as this tests also the filesystem cache, but having an option is good. Greetings Benjamin

@a1exsh
Copy link
Author

a1exsh commented Sep 11, 2015

Well, I don't see a benefit in testing the cache, but an option makes sense.

Unfortunately, normal os.read() will fail on file opened in direct mode due
to memory alignment requirement of the system call, so we have to use
directio module.

Cheers!
Alex

On Thu, Sep 10, 2015, 22:36 Benjamin Schweizer notifications@github.com
wrote:

Hi, direct io mode should be optional it should use os.open and not a 3rd
party module; not using direct io is intended as this tests also the
filesystem cache, but having an option is good. Greetings Benjamin


Reply to this email directly or view it on GitHub
#3 (comment).

@benschweizer
Copy link
Owner

Hi Alex,

to test real world scenarios, testing the cache is fine. So if you can make
this an option and handle import errors, that would be fine.

Greetings
Benjamin

On Fri, Sep 11, 2015 at 7:05 AM, a1exsh notifications@github.com wrote:

Well, I don't see a benefit in testing the cache, but an option makes
sense.

Unfortunately, normal os.read() will fail on file opened in direct mode due
to memory alignment requirement of the system call, so we have to use
directio module.

Cheers!
Alex

On Thu, Sep 10, 2015, 22:36 Benjamin Schweizer notifications@github.com
wrote:

Hi, direct io mode should be optional it should use os.open and not a 3rd
party module; not using direct io is intended as this tests also the
filesystem cache, but having an option is good. Greetings Benjamin


Reply to this email directly or view it on GitHub
#3 (comment).


Reply to this email directly or view it on GitHub
#3 (comment).

@a1exsh
Copy link
Author

a1exsh commented Sep 11, 2015

On Fri, Sep 11, 2015 at 9:05 PM, Benjamin Schweizer <
notifications@github.com> wrote:

Hi Alex,

to test real world scenarios, testing the cache is fine. So if you can make
this an option and handle import errors, that would be fine.

Please see #5. Not sure if handling import error makes a lot of sense: if
the the user didn't ask for direct IO with the option, it's not used, if
it's not installed or not supported the import error is sort of
self-describing.

Alex

@benschweizer
Copy link
Owner

Ok, let's decide this later;-) It won't do any damage when the option for direct io is off by default

@a1exsh
Copy link
Author

a1exsh commented Sep 11, 2015

On Fri, Sep 11, 2015 at 9:14 PM, Benjamin Schweizer <
notifications@github.com> wrote:

Ok, let's decide this later;-) It won't do any damage when the option for
direct io is off by default

Yeah, probably a mention that for the option to work, the directio module
needs to be installed, in the help message or readme is enough.

benschweizer added a commit that referenced this pull request Apr 15, 2016
stuejho pushed a commit to stuejho/iops that referenced this pull request Feb 19, 2023
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.

2 participants