Re: Review Request 58360: Added a test for evolving large protobuf message.

[Anand Mazumdar]

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


Fix it, then Ship it!




I would fix this minor thing before committing.


src/tests/protobuf_utils_tests.cpp
Lines 307 (patched)


Extra new line.


- Anand Mazumdar


On May 19, 2017, 5:57 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58360/
> ---
> 
> (Updated May 19, 2017, 5:57 p.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Bugs: MESOS-6644 and MESOS-7228
> https://issues.apache.org/jira/browse/MESOS-6644
> https://issues.apache.org/jira/browse/MESOS-7228
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Before protobuf 3.3.0, this test would fail because the 64MB limit
> imposed by older version of protobuf.
> 
> 
> Diffs
> -
> 
>   src/tests/protobuf_utils_tests.cpp 5239182812835b93a28e85146b2df2b20ae77328 
> 
> 
> Diff: https://reviews.apache.org/r/58360/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 58360: Added a test for evolving large protobuf message.

[Zhitao Li]

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

(Updated May 19, 2017, 5:57 p.m.)


Review request for mesos and Anand Mazumdar.


Bugs: MESOS-6644 and MESOS-7228
https://issues.apache.org/jira/browse/MESOS-6644
https://issues.apache.org/jira/browse/MESOS-7228


Repository: mesos


Description (updated)
---

Before protobuf 3.3.0, this test would fail because the 64MB limit
imposed by older version of protobuf.


Diffs (updated)
-

  src/tests/protobuf_utils_tests.cpp 5239182812835b93a28e85146b2df2b20ae77328 


Diff: https://reviews.apache.org/r/58360/diff/6/

Changes: https://reviews.apache.org/r/58360/diff/5-6/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 58360: Added a test for evolving large protobuf message.

[Zhitao Li]

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

(Updated May 17, 2017, 3:50 a.m.)


Review request for mesos and Anand Mazumdar.


Changes
---

Rebase.


Bugs: MESOS-6644 and MESOS-7228
https://issues.apache.org/jira/browse/MESOS-6644
https://issues.apache.org/jira/browse/MESOS-7228


Repository: mesos


Description
---

Before protobuf 3.2.0, this test would fail because the 64MB limit
imposed by older version of protobuf.


Diffs (updated)
-

  src/tests/protobuf_utils_tests.cpp 5239182812835b93a28e85146b2df2b20ae77328 


Diff: https://reviews.apache.org/r/58360/diff/4/

Changes: https://reviews.apache.org/r/58360/diff/3-4/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 58360: Added a test for evolving large protobuf message.

[Zhitao Li]

> On April 19, 2017, 5:07 p.m., Anand Mazumdar wrote:
> > src/tests/protobuf_utils_tests.cpp
> > Lines 289 (patched)
> > 
> >
> > Can you add a comment around why we added this test?
> 
> Chun-Hung Hsiao wrote:
> It seems to me that if we don't want to enforce a hard dependency on 
> protobuf-3.x, we should not add it.

I don't mind taking out the test, but I definitely need to make sure we are 
committed to supporting large message (>64MB).

The only alternative I can see is to ensure we have proper API design to 
paginate any potential protobuf message which could go large within the 64MB 
limit imposed by protobuf2.

AFAIK both `GET_STATE` and `OFFERS` messages could exceed this and render the 
cluster not usable for an important use case.

Do you have any concern about a hard dependency to protobuf 3.x?


- Zhitao


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


On April 19, 2017, 4:23 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58360/
> ---
> 
> (Updated April 19, 2017, 4:23 p.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Bugs: MESOS-6644 and MESOS-7228
> https://issues.apache.org/jira/browse/MESOS-6644
> https://issues.apache.org/jira/browse/MESOS-7228
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Before protobuf 3.2.0, this test would fail because the 64MB limit
> imposed by older version of protobuf.
> 
> 
> Diffs
> -
> 
>   src/tests/protobuf_utils_tests.cpp 5239182812835b93a28e85146b2df2b20ae77328 
> 
> 
> Diff: https://reviews.apache.org/r/58360/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 58360: Added a test for evolving large protobuf message.

[Chun-Hung Hsiao]

> On April 19, 2017, 5:07 p.m., Anand Mazumdar wrote:
> > src/tests/protobuf_utils_tests.cpp
> > Lines 289 (patched)
> > 
> >
> > Can you add a comment around why we added this test?

It seems to me that if we don't want to enforce a hard dependency on 
protobuf-3.x, we should not add it.


- Chun-Hung


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


On April 19, 2017, 4:23 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58360/
> ---
> 
> (Updated April 19, 2017, 4:23 p.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Bugs: MESOS-6644 and MESOS-7228
> https://issues.apache.org/jira/browse/MESOS-6644
> https://issues.apache.org/jira/browse/MESOS-7228
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Before protobuf 3.2.0, this test would fail because the 64MB limit
> imposed by older version of protobuf.
> 
> 
> Diffs
> -
> 
>   src/tests/protobuf_utils_tests.cpp 5239182812835b93a28e85146b2df2b20ae77328 
> 
> 
> Diff: https://reviews.apache.org/r/58360/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 58360: Added a test for evolving large protobuf message.

[Anand Mazumdar]

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




src/tests/protobuf_utils_tests.cpp
Lines 289 (patched)


Can you add a comment around why we added this test?



src/tests/protobuf_utils_tests.cpp
Lines 309 (patched)


Just do:

```cpp
evolve(executorInfo);
```


- Anand Mazumdar


On April 19, 2017, 4:23 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58360/
> ---
> 
> (Updated April 19, 2017, 4:23 p.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Bugs: MESOS-6644 and MESOS-7228
> https://issues.apache.org/jira/browse/MESOS-6644
> https://issues.apache.org/jira/browse/MESOS-7228
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Before protobuf 3.2.0, this test would fail because the 64MB limit
> imposed by older version of protobuf.
> 
> 
> Diffs
> -
> 
>   src/tests/protobuf_utils_tests.cpp 5239182812835b93a28e85146b2df2b20ae77328 
> 
> 
> Diff: https://reviews.apache.org/r/58360/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 58360: Added a test for evolving large protobuf message.

[Zhitao Li]

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

(Updated April 19, 2017, 4:23 p.m.)


Review request for mesos and Anand Mazumdar.


Bugs: MESOS-6644 and MESOS-7228
https://issues.apache.org/jira/browse/MESOS-6644
https://issues.apache.org/jira/browse/MESOS-7228


Repository: mesos


Description (updated)
---

Before protobuf 3.2.0, this test would fail because the 64MB limit
imposed by older version of protobuf.


Diffs (updated)
-

  src/tests/protobuf_utils_tests.cpp 5239182812835b93a28e85146b2df2b20ae77328 


Diff: https://reviews.apache.org/r/58360/diff/3/

Changes: https://reviews.apache.org/r/58360/diff/2-3/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 58360: Added a test for evolving large protobuf message.

[Zhitao Li]

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

(Updated April 18, 2017, 6:25 p.m.)


Review request for mesos and Anand Mazumdar.


Changes
---

Rebase.


Bugs: MESOS-6644 and MESOS-7228
https://issues.apache.org/jira/browse/MESOS-6644
https://issues.apache.org/jira/browse/MESOS-7228


Repository: mesos


Description
---

Before protobuf 3.2.0, this test would fail because the 64MB limit imposed by
older version of protobuf.


Diffs (updated)
-

  src/tests/protobuf_utils_tests.cpp 5239182812835b93a28e85146b2df2b20ae77328 


Diff: https://reviews.apache.org/r/58360/diff/2/

Changes: https://reviews.apache.org/r/58360/diff/1-2/


Testing
---


Thanks,

Zhitao Li