You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by tsutsu <gi...@git.apache.org> on 2015/09/29 01:17:01 UTC
[GitHub] couchdb-couch pull request: Inherit io_priority from the process c...
GitHub user tsutsu opened a pull request:
https://github.com/apache/couchdb-couch/pull/105
Inherit io_priority from the process creating the stream
`couch_stream` should be able to reuse the `io_priority` of whatever process creates it; it's then the creator's responsibility to decide what `io_priority` the stream should have.
Assigning streaming IO operations their own separately-tunable category of `io_priority` is an alternative to this change. The usefulness of that would depend on whether streaming IO in general has predictable load impacts that would be useful to observe and control for. Since `couch_stream` is only currently used by `couch_att`, though, giving attachment upload operations specifically an `io_priority` category might make more sense.
COUCHDB-2828
BugzID: 25815
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/cloudant/couchdb-couch 25815-couch-stream-io-priority
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/couchdb-couch/pull/105.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #105
----
commit fc3a8e2f3ef1784cb592aedf9733c7d8b7c2106d
Author: Levi McAuley <le...@leviaul.com>
Date: 2015-09-28T22:20:18Z
Inherit io_priority from the process creating the stream
COUCHDB-2828
BugzID: 25815
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Inherit io_priority from the process c...
Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/105#discussion_r40784314
--- Diff: src/couch_stream.erl ---
@@ -51,7 +51,7 @@ open(Fd) ->
open(Fd, []).
open(Fd, Options) ->
- gen_server:start_link(couch_stream, {Fd, self(), Options}, []).
+ gen_server:start_link(couch_stream, {Fd, {self(), erlang:get(io_priority)}, Options}, []).
--- End diff --
the init function would crash either way, so I think it's clearer if each required argument is expressed naturally.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Inherit io_priority from the process c...
Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:
https://github.com/apache/couchdb-couch/pull/105#issuecomment-144686032
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Inherit io_priority from the process c...
Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/105#discussion_r40653624
--- Diff: src/couch_stream.erl ---
@@ -51,7 +51,7 @@ open(Fd) ->
open(Fd, []).
open(Fd, Options) ->
- gen_server:start_link(couch_stream, {Fd, self(), Options}, []).
+ gen_server:start_link(couch_stream, {Fd, {self(), erlang:get(io_priority)}, Options}, []).
--- End diff --
why the extra nesting?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Inherit io_priority from the process c...
Posted by tsutsu <gi...@git.apache.org>.
Github user tsutsu commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/105#discussion_r40709916
--- Diff: src/couch_stream.erl ---
@@ -51,7 +51,7 @@ open(Fd) ->
open(Fd, []).
open(Fd, Options) ->
- gen_server:start_link(couch_stream, {Fd, self(), Options}, []).
+ gen_server:start_link(couch_stream, {Fd, {self(), erlang:get(io_priority)}, Options}, []).
--- End diff --
It's a bit of semantic idealism. Since `self()` is already being passed, and the gen_server could theoretically extract the `io_priority` from the passed pid() later on (using `erlang:process_info(Pid, dictionary)`, though it's inefficient and bad form), you could say that self() "contains" the io_priority in an OOP-theoretic sense. Effectively, the result of using `erlang:get/1` is *derived data* of `self()`, equivalent to something like the computed length of a list.
Frequently, you want to pass an object around along with its derived data, in such a way that its consumers think of the composite as an ADT with the same functionality as the original object. In a class-based language, you'd do something silly here with a decorator proxy to enable the object you're passing to respond to a `get_io_priority` message, but otherwise behave the same. In Erlang, you'd either use a record (if you want to create a contract on the shape of the ADT), or simply stick the original data together with its derived data into a tuple.
...pragmatically, though, that's a bunch of hot air and it's an extra structure/destructure op and could just as well be eliminated.
(It *would* have more practical value if any intermediate function calls occurred that had just expected "an owner", where the composite `{OwnerPid, OwnerPriority}` could "ride through" as an abstract `Owner` and then be unwrapped only by functions that care. But none do.)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Inherit io_priority from the process c...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/couchdb-couch/pull/105
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Inherit io_priority from the process c...
Posted by tsutsu <gi...@git.apache.org>.
Github user tsutsu commented on the pull request:
https://github.com/apache/couchdb-couch/pull/105#issuecomment-144500060
Nesting removed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---