You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Ben Mahler <be...@gmail.com> on 2015/07/22 08:09:58 UTC

Review Request 36677: Introduced 'recordio' encoding facilities to stout.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36677/
-----------------------------------------------------------

Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Jie Yu.


Bugs: MESOS-3067
    https://issues.apache.org/jira/browse/MESOS-3067


Repository: mesos


Description
-------

Note that most "Record-IO" encodings are used for file I/O
and consequently use a fixed-size header to encode the record
length. However, decoding a base-10 integer is more
straightforward to implement in most languages, and so this
was chosen instead. (Note that the Twitter streaming API
uses the same technique for portability).


Diffs
-----

  3rdparty/libprocess/3rdparty/Makefile.am 856c2b289451fd404b97285b825e72913feb2f04 
  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 2394b95462182273464f0847f416ad83c3b64485 
  3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 8c75f6b28c18596018eaefe427b238424aae2fd9 
  3rdparty/libprocess/3rdparty/stout/include/stout/recordio.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/tests/recordio_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/36677/diff/


Testing
-------

Added tests.


Thanks,

Ben Mahler


Re: Review Request 36677: Introduced 'recordio' encoding facilities to stout.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36677/#review92556
-----------------------------------------------------------


Bad patch!

Reviews applied: [36677]

Failed command: ./support/apply-review.sh -n -r 36677

Error:
 2015-07-22 06:28:31 URL:https://reviews.apache.org/r/36677/diff/raw/ [10398/10398] -> "36677.patch" [1]
Successfully applied: Introduced 'recordio' encoding facilities to stout.

Note that most "Record-IO" encodings are used for file I/O
and consequently use a fixed-size header to encode the record
length. However, decoding a base-10 integer is more
straightforward to implement in most languages, and so this
was chosen instead. (Note that the Twitter streaming API
uses the same technique for portability).


Review: https://reviews.apache.org/r/36677
Checking 3 files using filter --filter=-,+build/class,+build/deprecated,+build/endif_comment,+readability/todo,+readability/namespace,+runtime/vlog,+whitespace/blank_line,+whitespace/comma,+whitespace/end_of_line,+whitespace/ending_newline,+whitespace/forcolon,+whitespace/indent,+whitespace/line_length,+whitespace/operators,+whitespace/semicolon,+whitespace/tab,+whitespace/todo
Total errors found: 0
ERROR: Commit spanning multiple projects.

Please use separate commits for mesos, libprocess and stout.

Paths grouped by project:
stout:
  3rdparty/libprocess/3rdparty/stout/include/Makefile.am
  3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp
  3rdparty/libprocess/3rdparty/stout/include/stout/recordio.hpp
  3rdparty/libprocess/3rdparty/stout/tests/recordio_tests.cpp
libprocess:
  3rdparty/libprocess/3rdparty/Makefile.am
Failed to commit patch

- Mesos ReviewBot


On July 22, 2015, 6:09 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36677/
> -----------------------------------------------------------
> 
> (Updated July 22, 2015, 6:09 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Jie Yu.
> 
> 
> Bugs: MESOS-3067
>     https://issues.apache.org/jira/browse/MESOS-3067
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Note that most "Record-IO" encodings are used for file I/O
> and consequently use a fixed-size header to encode the record
> length. However, decoding a base-10 integer is more
> straightforward to implement in most languages, and so this
> was chosen instead. (Note that the Twitter streaming API
> uses the same technique for portability).
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/Makefile.am 856c2b289451fd404b97285b825e72913feb2f04 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 2394b95462182273464f0847f416ad83c3b64485 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 8c75f6b28c18596018eaefe427b238424aae2fd9 
>   3rdparty/libprocess/3rdparty/stout/include/stout/recordio.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/recordio_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36677/diff/
> 
> 
> Testing
> -------
> 
> Added tests.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 36677: Introduced 'recordio' encoding facilities to stout.

Posted by Ben Mahler <be...@gmail.com>.

> On July 22, 2015, 6:46 p.m., Vinod Kone wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/recordio.hpp, lines 54-61
> > <https://reviews.apache.org/r/36677/diff/2/?file=1018410#file1018410line54>
> >
> >     Add comments!

Added a javadoc at the top of encoder and decoder to capture that these wrap individual record encoding / decoding.


> On July 22, 2015, 6:46 p.m., Vinod Kone wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/recordio.hpp, line 83
> > <https://reviews.apache.org/r/36677/diff/2/?file=1018410#file1018410line83>
> >
> >     Can you add a comment about whether data can contain a partial record?

Re-phrased the documentation to make this more clear.


- Ben


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36677/#review92626
-----------------------------------------------------------


On July 22, 2015, 6:33 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36677/
> -----------------------------------------------------------
> 
> (Updated July 22, 2015, 6:33 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Jie Yu.
> 
> 
> Bugs: MESOS-3067
>     https://issues.apache.org/jira/browse/MESOS-3067
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Note that most "Record-IO" encodings are used for file I/O
> and consequently use a fixed-size header to encode the record
> length. However, decoding a base-10 integer is more
> straightforward to implement in most languages, and so this
> was chosen instead. (Note that the Twitter streaming API
> uses the same technique for portability).
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 2394b95462182273464f0847f416ad83c3b64485 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 8c75f6b28c18596018eaefe427b238424aae2fd9 
>   3rdparty/libprocess/3rdparty/stout/include/stout/recordio.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/recordio_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36677/diff/
> 
> 
> Testing
> -------
> 
> Added tests.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 36677: Introduced 'recordio' encoding facilities to stout.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36677/#review92626
-----------------------------------------------------------


nice tests!


3rdparty/libprocess/3rdparty/stout/include/stout/recordio.hpp (lines 54 - 61)
<https://reviews.apache.org/r/36677/#comment146834>

    Add comments!



3rdparty/libprocess/3rdparty/stout/include/stout/recordio.hpp (lines 72 - 73)
<https://reviews.apache.org/r/36677/#comment146836>

    Comment please.



3rdparty/libprocess/3rdparty/stout/include/stout/recordio.hpp (line 83)
<https://reviews.apache.org/r/36677/#comment146837>

    Can you add a comment about whether data can contain a partial record?


- Vinod Kone


On July 22, 2015, 6:33 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36677/
> -----------------------------------------------------------
> 
> (Updated July 22, 2015, 6:33 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Jie Yu.
> 
> 
> Bugs: MESOS-3067
>     https://issues.apache.org/jira/browse/MESOS-3067
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Note that most "Record-IO" encodings are used for file I/O
> and consequently use a fixed-size header to encode the record
> length. However, decoding a base-10 integer is more
> straightforward to implement in most languages, and so this
> was chosen instead. (Note that the Twitter streaming API
> uses the same technique for portability).
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 2394b95462182273464f0847f416ad83c3b64485 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 8c75f6b28c18596018eaefe427b238424aae2fd9 
>   3rdparty/libprocess/3rdparty/stout/include/stout/recordio.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/recordio_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36677/diff/
> 
> 
> Testing
> -------
> 
> Added tests.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 36677: Introduced 'recordio' encoding facilities to stout.

Posted by Benjamin Hindman <be...@berkeley.edu>.

> On July 23, 2015, 11:52 p.m., Benjamin Hindman wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/recordio_tests.cpp, line 32
> > <https://reviews.apache.org/r/36677/diff/3/?file=1019202#file1019202line32>
> >
> >     Why not add these to try.hpp like we did with Option?
> 
> Ben Mahler wrote:
>     Decided to punt because I'm not sure whether equality should include the error message equality, thoughts?

Hmm, good question. I might start by not testing for error message equality to deter that use case, and see if it becomes necessary later. Either way, a comment above these operators as to why we're keeping them here instead of pulling them to try.hpp would be very helpful for a future code reader!


- Benjamin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36677/#review92812
-----------------------------------------------------------


On July 22, 2015, 11:24 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36677/
> -----------------------------------------------------------
> 
> (Updated July 22, 2015, 11:24 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Jie Yu.
> 
> 
> Bugs: MESOS-3067
>     https://issues.apache.org/jira/browse/MESOS-3067
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Note that most "Record-IO" encodings are used for file I/O
> and consequently use a fixed-size header to encode the record
> length. However, decoding a base-10 integer is more
> straightforward to implement in most languages, and so this
> was chosen instead. (Note that the Twitter streaming API
> uses the same technique for portability).
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 2394b95462182273464f0847f416ad83c3b64485 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 8c75f6b28c18596018eaefe427b238424aae2fd9 
>   3rdparty/libprocess/3rdparty/stout/include/stout/recordio.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/recordio_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36677/diff/
> 
> 
> Testing
> -------
> 
> Added tests.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 36677: Introduced 'recordio' encoding facilities to stout.

Posted by Ben Mahler <be...@gmail.com>.

> On July 23, 2015, 11:52 p.m., Benjamin Hindman wrote:
> > The names 'encode' and 'decode' seem weird for the function parameters to Encoder and Decoder, since Encoder::encode and Decoder::decode (which are named the same thing) are really the things that are "encoding" and "decoding" the stream (i.e., adding the number and newline). These inner functions are more like 'serialize' and 'deserialize'? And by default shouldn't we just assume we'll take string to string?
> > 
> > Also, this is not async, but I guess we'll async read data that we plug into Decoder::Decode?

serialize and deserialize are much clearer, thanks Ben!

Yeah, it's not async for now, so we'll just plug in the data, but per my comment earlier I'll add a TODO for making it async.


> On July 23, 2015, 11:52 p.m., Benjamin Hindman wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/recordio_tests.cpp, line 32
> > <https://reviews.apache.org/r/36677/diff/3/?file=1019202#file1019202line32>
> >
> >     Why not add these to try.hpp like we did with Option?

Decided to punt because I'm not sure whether equality should include the error message equality, thoughts?


- Ben


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36677/#review92812
-----------------------------------------------------------


On July 22, 2015, 11:24 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36677/
> -----------------------------------------------------------
> 
> (Updated July 22, 2015, 11:24 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Jie Yu.
> 
> 
> Bugs: MESOS-3067
>     https://issues.apache.org/jira/browse/MESOS-3067
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Note that most "Record-IO" encodings are used for file I/O
> and consequently use a fixed-size header to encode the record
> length. However, decoding a base-10 integer is more
> straightforward to implement in most languages, and so this
> was chosen instead. (Note that the Twitter streaming API
> uses the same technique for portability).
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 2394b95462182273464f0847f416ad83c3b64485 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 8c75f6b28c18596018eaefe427b238424aae2fd9 
>   3rdparty/libprocess/3rdparty/stout/include/stout/recordio.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/recordio_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36677/diff/
> 
> 
> Testing
> -------
> 
> Added tests.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 36677: Introduced 'recordio' encoding facilities to stout.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36677/#review92812
-----------------------------------------------------------


The names 'encode' and 'decode' seem weird for the function parameters to Encoder and Decoder, since Encoder::encode and Decoder::decode (which are named the same thing) are really the things that are "encoding" and "decoding" the stream (i.e., adding the number and newline). These inner functions are more like 'serialize' and 'deserialize'? And by default shouldn't we just assume we'll take string to string?

Also, this is not async, but I guess we'll async read data that we plug into Decoder::Decode?


3rdparty/libprocess/3rdparty/stout/tests/recordio_tests.cpp (line 32)
<https://reviews.apache.org/r/36677/#comment147058>

    Why not add these to try.hpp like we did with Option?


- Benjamin Hindman


On July 22, 2015, 11:24 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36677/
> -----------------------------------------------------------
> 
> (Updated July 22, 2015, 11:24 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Jie Yu.
> 
> 
> Bugs: MESOS-3067
>     https://issues.apache.org/jira/browse/MESOS-3067
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Note that most "Record-IO" encodings are used for file I/O
> and consequently use a fixed-size header to encode the record
> length. However, decoding a base-10 integer is more
> straightforward to implement in most languages, and so this
> was chosen instead. (Note that the Twitter streaming API
> uses the same technique for portability).
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 2394b95462182273464f0847f416ad83c3b64485 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 8c75f6b28c18596018eaefe427b238424aae2fd9 
>   3rdparty/libprocess/3rdparty/stout/include/stout/recordio.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/recordio_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36677/diff/
> 
> 
> Testing
> -------
> 
> Added tests.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 36677: Introduced 'recordio' encoding facilities to stout.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36677/#review92690
-----------------------------------------------------------


I'll add a note about "completion" semantics (surface an error when the stream ends with a partial record). This is not immediately useful but note that it is provided by joyent's http parser: https://github.com/joyent/http-parser (search for EOF).


3rdparty/libprocess/3rdparty/stout/include/stout/recordio.hpp (line 29)
<https://reviews.apache.org/r/36677/#comment146918>

    I'll add a TODO here that it would be nice to compose asynchronous streams of data, e.g.:
    
    Stream<string> | Decoder<Event> -> Stream<Event>
    
    But we don't have the necessary abstractions yet in libprocess.



3rdparty/libprocess/3rdparty/stout/include/stout/recordio.hpp (line 156)
<https://reviews.apache.org/r/36677/#comment146917>

    Add a note about memory allocation here, the string will retain its maximum size after a .clear() call:
    
    http://stackoverflow.com/q/25974085


- Ben Mahler


On July 22, 2015, 11:24 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36677/
> -----------------------------------------------------------
> 
> (Updated July 22, 2015, 11:24 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Jie Yu.
> 
> 
> Bugs: MESOS-3067
>     https://issues.apache.org/jira/browse/MESOS-3067
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Note that most "Record-IO" encodings are used for file I/O
> and consequently use a fixed-size header to encode the record
> length. However, decoding a base-10 integer is more
> straightforward to implement in most languages, and so this
> was chosen instead. (Note that the Twitter streaming API
> uses the same technique for portability).
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 2394b95462182273464f0847f416ad83c3b64485 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 8c75f6b28c18596018eaefe427b238424aae2fd9 
>   3rdparty/libprocess/3rdparty/stout/include/stout/recordio.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/recordio_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36677/diff/
> 
> 
> Testing
> -------
> 
> Added tests.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 36677: Introduced 'recordio' encoding facilities to stout.

Posted by Ben Mahler <be...@gmail.com>.

> On July 23, 2015, 6:01 p.m., Jie Yu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/recordio.hpp, line 130
> > <https://reviews.apache.org/r/36677/diff/3/?file=1019201#file1019201line130>
> >
> >     Not needed?

Yeah, I've restructured the logic to be more clear (now we always transition to RECORD, but for 0 length we parse immediately and transition back to HEADER). Thanks jie!


- Ben


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36677/#review92627
-----------------------------------------------------------


On July 22, 2015, 11:24 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36677/
> -----------------------------------------------------------
> 
> (Updated July 22, 2015, 11:24 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Jie Yu.
> 
> 
> Bugs: MESOS-3067
>     https://issues.apache.org/jira/browse/MESOS-3067
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Note that most "Record-IO" encodings are used for file I/O
> and consequently use a fixed-size header to encode the record
> length. However, decoding a base-10 integer is more
> straightforward to implement in most languages, and so this
> was chosen instead. (Note that the Twitter streaming API
> uses the same technique for portability).
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 2394b95462182273464f0847f416ad83c3b64485 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 8c75f6b28c18596018eaefe427b238424aae2fd9 
>   3rdparty/libprocess/3rdparty/stout/include/stout/recordio.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/recordio_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36677/diff/
> 
> 
> Testing
> -------
> 
> Added tests.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 36677: Introduced 'recordio' encoding facilities to stout.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36677/#review92627
-----------------------------------------------------------

Ship it!


Nice test!


3rdparty/libprocess/3rdparty/stout/include/stout/recordio.hpp (lines 118 - 119)
<https://reviews.apache.org/r/36677/#comment146921>

    Can you fix the indentation:
    ```
    return Error("Failed ..." +
                 numify.error());
    ```
    or
    ```
    return Error(
        "Failed"... + ...
         numify.error()
    ```



3rdparty/libprocess/3rdparty/stout/include/stout/recordio.hpp (line 129)
<https://reviews.apache.org/r/36677/#comment147018>

    Maybe do the following which is more explicit?
    ```
    records.push_back(decode_(""));
    ```



3rdparty/libprocess/3rdparty/stout/include/stout/recordio.hpp (line 130)
<https://reviews.apache.org/r/36677/#comment147019>

    Not needed?


- Jie Yu


On July 22, 2015, 11:24 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36677/
> -----------------------------------------------------------
> 
> (Updated July 22, 2015, 11:24 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Jie Yu.
> 
> 
> Bugs: MESOS-3067
>     https://issues.apache.org/jira/browse/MESOS-3067
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Note that most "Record-IO" encodings are used for file I/O
> and consequently use a fixed-size header to encode the record
> length. However, decoding a base-10 integer is more
> straightforward to implement in most languages, and so this
> was chosen instead. (Note that the Twitter streaming API
> uses the same technique for portability).
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 2394b95462182273464f0847f416ad83c3b64485 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 8c75f6b28c18596018eaefe427b238424aae2fd9 
>   3rdparty/libprocess/3rdparty/stout/include/stout/recordio.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/recordio_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36677/diff/
> 
> 
> Testing
> -------
> 
> Added tests.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request 36677: Introduced 'recordio' encoding facilities to stout.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36677/
-----------------------------------------------------------

(Updated July 22, 2015, 11:24 p.m.)


Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Jie Yu.


Changes
-------

Added documentation.


Bugs: MESOS-3067
    https://issues.apache.org/jira/browse/MESOS-3067


Repository: mesos


Description
-------

Note that most "Record-IO" encodings are used for file I/O
and consequently use a fixed-size header to encode the record
length. However, decoding a base-10 integer is more
straightforward to implement in most languages, and so this
was chosen instead. (Note that the Twitter streaming API
uses the same technique for portability).


Diffs (updated)
-----

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 2394b95462182273464f0847f416ad83c3b64485 
  3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 8c75f6b28c18596018eaefe427b238424aae2fd9 
  3rdparty/libprocess/3rdparty/stout/include/stout/recordio.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/tests/recordio_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/36677/diff/


Testing
-------

Added tests.


Thanks,

Ben Mahler


Re: Review Request 36677: Introduced 'recordio' encoding facilities to stout.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36677/
-----------------------------------------------------------

(Updated July 22, 2015, 6:33 a.m.)


Review request for mesos, Anand Mazumdar, Benjamin Hindman, and Jie Yu.


Changes
-------

Removed libprocess code.


Bugs: MESOS-3067
    https://issues.apache.org/jira/browse/MESOS-3067


Repository: mesos


Description
-------

Note that most "Record-IO" encodings are used for file I/O
and consequently use a fixed-size header to encode the record
length. However, decoding a base-10 integer is more
straightforward to implement in most languages, and so this
was chosen instead. (Note that the Twitter streaming API
uses the same technique for portability).


Diffs (updated)
-----

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 2394b95462182273464f0847f416ad83c3b64485 
  3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 8c75f6b28c18596018eaefe427b238424aae2fd9 
  3rdparty/libprocess/3rdparty/stout/include/stout/recordio.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/tests/recordio_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/36677/diff/


Testing
-------

Added tests.


Thanks,

Ben Mahler