Re: Review Request 37655: MESOS-3299: Add protobuf to represent time with integer precision.
On Aug. 27, 2015, 5:34 a.m., Neil Conway wrote: include/mesos/mesos.proto, line 102 https://reviews.apache.org/r/37655/diff/2/?file=1049806#file1049806line102 BTW, have you considered using a simpler representation, such as just a single int64 holding # of nanoseconds (or microseconds) since the Unix epoch? Using nanoseconds you'd be able to express ~292 years -- maybe that's not a wide enough range? Joseph Wu wrote: We were aiming at something that could be used like the `struct timespec`. So rather than the range of times that can be represented, we were looking for usability (would you rather think in terms of seconds or millions of nanoseconds?); with the option for extra precision. (It would be somewhat simpler, code-wise, to just use nanoseconds.) Fair enough. For what it is worth, personally I find a single field to be more usable than an embedded message with two fields, one of which is optional. If/when humans are generating/reading messages, I agree that seconds would be simpler. - Neil --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37655/#review96648 --- On Aug. 26, 2015, 10:56 p.m., Joseph Wu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37655/ --- (Updated Aug. 26, 2015, 10:56 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-3299 https://issues.apache.org/jira/browse/MESOS-3299 Repository: mesos Description --- Instead of using doubles, seconds and nanoseconds can be represented like `struct timespec`, with one field for seconds and one for nanoseconds. This will be important if frameworks need to compare times to make decisions (such as for maintenance primitives). Note about the naming: * Time will conflict with the Time class. * Most denominations of time (Seconds, Minutes, Hours, etc) will conflict with Duration. Diffs - include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 include/mesos/v1/mesos.proto 382b978dca769757171c5558b7f259870592c321 Diff: https://reviews.apache.org/r/37655/diff/ Testing --- `make check` Thanks, Joseph Wu
Re: Review Request 37655: MESOS-3299: Add protobuf to represent time with integer precision.
On Aug. 26, 2015, 10:34 p.m., Neil Conway wrote: Thanks for your comments! On Aug. 26, 2015, 10:34 p.m., Neil Conway wrote: include/mesos/mesos.proto, line 101 https://reviews.apache.org/r/37655/diff/2/?file=1049806#file1049806line101 It might be useful to specify what the epoch is (e.g., seconds-since-Unix-epoch). The `seconds` field actually just expresses seconds (like a `Duration` object, rather than a `Time` object). In the next review in this chain, we use this protobuf twice, once as epoch seconds and once as a duration. On Aug. 26, 2015, 10:34 p.m., Neil Conway wrote: include/mesos/mesos.proto, line 102 https://reviews.apache.org/r/37655/diff/2/?file=1049806#file1049806line102 BTW, have you considered using a simpler representation, such as just a single int64 holding # of nanoseconds (or microseconds) since the Unix epoch? Using nanoseconds you'd be able to express ~292 years -- maybe that's not a wide enough range? We were aiming at something that could be used like the `struct timespec`. So rather than the range of times that can be represented, we were looking for usability (would you rather think in terms of seconds or millions of nanoseconds?); with the option for extra precision. (It would be somewhat simpler, code-wise, to just use nanoseconds.) - Joseph --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37655/#review96648 --- On Aug. 26, 2015, 3:56 p.m., Joseph Wu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37655/ --- (Updated Aug. 26, 2015, 3:56 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-3299 https://issues.apache.org/jira/browse/MESOS-3299 Repository: mesos Description --- Instead of using doubles, seconds and nanoseconds can be represented like `struct timespec`, with one field for seconds and one for nanoseconds. This will be important if frameworks need to compare times to make decisions (such as for maintenance primitives). Note about the naming: * Time will conflict with the Time class. * Most denominations of time (Seconds, Minutes, Hours, etc) will conflict with Duration. Diffs - include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 include/mesos/v1/mesos.proto 382b978dca769757171c5558b7f259870592c321 Diff: https://reviews.apache.org/r/37655/diff/ Testing --- `make check` Thanks, Joseph Wu
Re: Review Request 37655: MESOS-3299: Add protobuf to represent time with integer precision.
On Aug. 26, 2015, 11 p.m., Alexander Rukletsov wrote: Do you plan to migrate existing protobufs to `TimeSpec` or use it for new protobufs only? For now, the plan is to have new protobufs use it. And we can change existing protobufs to use it, if they benefit from having integer time. - Joseph --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37655/#review96638 --- On Aug. 26, 2015, 3:56 p.m., Joseph Wu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37655/ --- (Updated Aug. 26, 2015, 3:56 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-3299 https://issues.apache.org/jira/browse/MESOS-3299 Repository: mesos Description --- Instead of using doubles, seconds and nanoseconds can be represented like `struct timespec`, with one field for seconds and one for nanoseconds. This will be important if frameworks need to compare times to make decisions (such as for maintenance primitives). Note about the naming: * Time will conflict with the Time class. * Most denominations of time (Seconds, Minutes, Hours, etc) will conflict with Duration. Diffs - include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 include/mesos/v1/mesos.proto 382b978dca769757171c5558b7f259870592c321 Diff: https://reviews.apache.org/r/37655/diff/ Testing --- `make check` Thanks, Joseph Wu
Re: Review Request 37655: MESOS-3299: Add protobuf to represent time with integer precision.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37655/ --- (Updated Aug. 27, 2015, 1:14 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Changes --- Break TimeSpec into TimeInfo and DurationInfo, so as to explicitly form the type wherever it is used. Bugs: MESOS-3299 https://issues.apache.org/jira/browse/MESOS-3299 Repository: mesos Description (updated) --- Instead of using doubles, time and duration can be represented as integers for nanoseconds. This will be important if frameworks need to compare times to make decisions (such as for maintenance primitives). Note about the naming: * TimeInfo will directly correspond with the Time class. * DurationInfo will directly correspond with the Duration class. Diffs (updated) - include/mesos/mesos.proto 715b8cf38e1e56c18a3f2ddbb82c920bd9414f05 include/mesos/v1/mesos.proto 382b978dca769757171c5558b7f259870592c321 Diff: https://reviews.apache.org/r/37655/diff/ Testing --- `make check` Thanks, Joseph Wu
Re: Review Request 37655: MESOS-3299: Add protobuf to represent time with integer precision.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37655/ --- (Updated Aug. 27, 2015, 3:24 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Changes --- Add TODO for TimeInfo time zones. Bugs: MESOS-3299 https://issues.apache.org/jira/browse/MESOS-3299 Repository: mesos Description --- Instead of using doubles, time and duration can be represented as integers for nanoseconds. This will be important if frameworks need to compare times to make decisions (such as for maintenance primitives). Note about the naming: * TimeInfo will directly correspond with the Time class. * DurationInfo will directly correspond with the Duration class. Diffs (updated) - include/mesos/mesos.proto 715b8cf38e1e56c18a3f2ddbb82c920bd9414f05 include/mesos/v1/mesos.proto 382b978dca769757171c5558b7f259870592c321 Diff: https://reviews.apache.org/r/37655/diff/ Testing --- `make check` Thanks, Joseph Wu
Re: Review Request 37655: MESOS-3299: Add protobuf to represent time with integer precision.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37655/#review96778 --- include/mesos/mesos.proto (line 97) https://reviews.apache.org/r/37655/#comment152426 This will overflow in less than 247 years. Be prepared to find a different patch by then! :-) - Bernd Mathiske On Aug. 27, 2015, 1:14 p.m., Joseph Wu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37655/ --- (Updated Aug. 27, 2015, 1:14 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-3299 https://issues.apache.org/jira/browse/MESOS-3299 Repository: mesos Description --- Instead of using doubles, time and duration can be represented as integers for nanoseconds. This will be important if frameworks need to compare times to make decisions (such as for maintenance primitives). Note about the naming: * TimeInfo will directly correspond with the Time class. * DurationInfo will directly correspond with the Duration class. Diffs - include/mesos/mesos.proto 715b8cf38e1e56c18a3f2ddbb82c920bd9414f05 include/mesos/v1/mesos.proto 382b978dca769757171c5558b7f259870592c321 Diff: https://reviews.apache.org/r/37655/diff/ Testing --- `make check` Thanks, Joseph Wu
Re: Review Request 37655: MESOS-3299: Add protobuf to represent time with integer precision.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37655/#review96648 --- include/mesos/mesos.proto (line 101) https://reviews.apache.org/r/37655/#comment152260 It might be useful to specify what the epoch is (e.g., seconds-since-Unix-epoch). include/mesos/mesos.proto (line 102) https://reviews.apache.org/r/37655/#comment152261 BTW, have you considered using a simpler representation, such as just a single int64 holding # of nanoseconds (or microseconds) since the Unix epoch? Using nanoseconds you'd be able to express ~292 years -- maybe that's not a wide enough range? - Neil Conway On Aug. 26, 2015, 10:56 p.m., Joseph Wu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37655/ --- (Updated Aug. 26, 2015, 10:56 p.m.) Review request for mesos, Benjamin Hindman, Bernd Mathiske, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-3299 https://issues.apache.org/jira/browse/MESOS-3299 Repository: mesos Description --- Instead of using doubles, seconds and nanoseconds can be represented like `struct timespec`, with one field for seconds and one for nanoseconds. This will be important if frameworks need to compare times to make decisions (such as for maintenance primitives). Note about the naming: * Time will conflict with the Time class. * Most denominations of time (Seconds, Minutes, Hours, etc) will conflict with Duration. Diffs - include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 include/mesos/v1/mesos.proto 382b978dca769757171c5558b7f259870592c321 Diff: https://reviews.apache.org/r/37655/diff/ Testing --- `make check` Thanks, Joseph Wu
Re: Review Request 37655: MESOS-3299: Add protobuf to represent time with integer precision.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37655/#review96244 --- Ship it! Ship It! include/mesos/mesos.proto (line 98) https://reviews.apache.org/r/37655/#comment151584 Can you expand the comment to make it clean that nanoseconds is the remainder, not a more precise representation of the full amount of time. - Joris Van Remoortere On Aug. 20, 2015, 5:03 p.m., Joseph Wu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37655/ --- (Updated Aug. 20, 2015, 5:03 p.m.) Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-3299 https://issues.apache.org/jira/browse/MESOS-3299 Repository: mesos Description --- Instead of using doubles, seconds and nanoseconds can be represented like `struct timespec`, with one field for seconds and one for nanoseconds. This will be important if frameworks need to compare times to make decisions (such as for maintenance primitives). Note about the naming: * Time will conflict with the Time class. * Most denominations of time (Seconds, Minutes, Hours, etc) will conflict with Duration. Diffs - include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 Diff: https://reviews.apache.org/r/37655/diff/ Testing --- `make check` Thanks, Joseph Wu
Re: Review Request 37655: MESOS-3299: Add protobuf to represent time with integer precision.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37655/ --- (Updated Aug. 24, 2015, 5:14 p.m.) Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Changes --- Update comments. Copy to V1 API too. Bugs: MESOS-3299 https://issues.apache.org/jira/browse/MESOS-3299 Repository: mesos Description --- Instead of using doubles, seconds and nanoseconds can be represented like `struct timespec`, with one field for seconds and one for nanoseconds. This will be important if frameworks need to compare times to make decisions (such as for maintenance primitives). Note about the naming: * Time will conflict with the Time class. * Most denominations of time (Seconds, Minutes, Hours, etc) will conflict with Duration. Diffs (updated) - include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 include/mesos/v1/mesos.proto 382b978dca769757171c5558b7f259870592c321 Diff: https://reviews.apache.org/r/37655/diff/ Testing --- `make check` Thanks, Joseph Wu
Re: Review Request 37655: MESOS-3299: Add protobuf to represent time with integer precision.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37655/#review96269 --- Ship it! Ship It! - Guangya Liu On Aug. 25, 2015, 12:14 a.m., Joseph Wu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37655/ --- (Updated Aug. 25, 2015, 12:14 a.m.) Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-3299 https://issues.apache.org/jira/browse/MESOS-3299 Repository: mesos Description --- Instead of using doubles, seconds and nanoseconds can be represented like `struct timespec`, with one field for seconds and one for nanoseconds. This will be important if frameworks need to compare times to make decisions (such as for maintenance primitives). Note about the naming: * Time will conflict with the Time class. * Most denominations of time (Seconds, Minutes, Hours, etc) will conflict with Duration. Diffs - include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 include/mesos/v1/mesos.proto 382b978dca769757171c5558b7f259870592c321 Diff: https://reviews.apache.org/r/37655/diff/ Testing --- `make check` Thanks, Joseph Wu
Re: Review Request 37655: MESOS-3299: Add protobuf to represent time with integer precision.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37655/#review96285 --- Patch looks great! Reviews applied: [37655] All tests passed. - Mesos ReviewBot On Aug. 25, 2015, 12:14 a.m., Joseph Wu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37655/ --- (Updated Aug. 25, 2015, 12:14 a.m.) Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-3299 https://issues.apache.org/jira/browse/MESOS-3299 Repository: mesos Description --- Instead of using doubles, seconds and nanoseconds can be represented like `struct timespec`, with one field for seconds and one for nanoseconds. This will be important if frameworks need to compare times to make decisions (such as for maintenance primitives). Note about the naming: * Time will conflict with the Time class. * Most denominations of time (Seconds, Minutes, Hours, etc) will conflict with Duration. Diffs - include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 include/mesos/v1/mesos.proto 382b978dca769757171c5558b7f259870592c321 Diff: https://reviews.apache.org/r/37655/diff/ Testing --- `make check` Thanks, Joseph Wu
Re: Review Request 37655: MESOS-3299: Add protobuf to represent time with integer precision.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37655/ --- (Updated Aug. 20, 2015, 10:03 a.m.) Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-3299 https://issues.apache.org/jira/browse/MESOS-3299 Repository: mesos Description (updated) --- Instead of using doubles, seconds and nanoseconds can be represented like `struct timespec`, with one field for seconds and one for nanoseconds. This will be important if frameworks need to compare times to make decisions (such as for maintenance primitives). Note about the naming: * Time will conflict with the Time class. * Most denominations of time (Seconds, Minutes, Hours, etc) will conflict with Duration. Diffs - include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 Diff: https://reviews.apache.org/r/37655/diff/ Testing --- `make check` Thanks, Joseph Wu
Re: Review Request 37655: MESOS-3299: Add protobuf to represent time with integer precision.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37655/#review95981 --- Patch looks great! Reviews applied: [37655] All tests passed. - Mesos ReviewBot On Aug. 20, 2015, 5:03 p.m., Joseph Wu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37655/ --- (Updated Aug. 20, 2015, 5:03 p.m.) Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-3299 https://issues.apache.org/jira/browse/MESOS-3299 Repository: mesos Description --- Instead of using doubles, seconds and nanoseconds can be represented like `struct timespec`, with one field for seconds and one for nanoseconds. This will be important if frameworks need to compare times to make decisions (such as for maintenance primitives). Note about the naming: * Time will conflict with the Time class. * Most denominations of time (Seconds, Minutes, Hours, etc) will conflict with Duration. Diffs - include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 Diff: https://reviews.apache.org/r/37655/diff/ Testing --- `make check` Thanks, Joseph Wu
