> On Feb. 4, 2013, 9:15 p.m., Benjamin Hindman wrote: > > include/mesos/mesos.proto, line 130 > > <https://reviews.apache.org/r/9089/diff/2/?file=252480#file252480line130> > > > > You should always be using increasing numbers to identify new fields. > > It's highly likely that '2' was used before. While it's less likely that > > there is still a version of the code with '2' being run when we (or someone > > else) upgrades, there's not reason to run that risk. Likewise, I'd move > > 'name' down below resources since it's optional and the others are required > > (or at least 'framework_id' should be required, see comment). Plus it's > > nice to keep things related together (the two IDs in this case). Finally, > > let's update the comment to explain the value of 'name', in particular that > > the aggregate resource usage for the executor and all > > tasks/threads/processes underneath it will be exposed via JSON on the slave > > so that users can import that information into their own TSDB for > > monitoring/observability.
Using 9 now, moved name down, and added a comment. > On Feb. 4, 2013, 9:15 p.m., Benjamin Hindman wrote: > > src/slave/http.cpp, line 149 > > <https://reviews.apache.org/r/9089/diff/2/?file=252486#file252486line149> > > > > Does the "blockers before 1.0" JIRA include switching to your protobuf > > -> JSON converter? Because it should! Added to that ticket. > On Feb. 4, 2013, 9:15 p.m., Benjamin Hindman wrote: > > src/examples/long_lived_framework.cpp, line 170 > > <https://reviews.apache.org/r/9089/diff/2/?file=252482#file252482line170> > > > > Just because I'm so pedantic, I would update the order you set the name > > to match the order in the proto file (I know it matches now, but given my > > comment above, if it moves, you can update here and other spots too). done here and elsewhere - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9089/#review16065 ----------------------------------------------------------- On Jan. 28, 2013, 10:41 p.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/9089/ > ----------------------------------------------------------- > > (Updated Jan. 28, 2013, 10:41 p.m.) > > > Review request for mesos, Benjamin Hindman and Vinod Kone. > > > Description > ------- > > This adds an executor name into ExecutorInfo. > > This name is useful to humans and frameworks. In particular, this proves > useful at Twitter to further identify executors for resource monitoring. > > > This addresses bug MESOS-324. > https://issues.apache.org/jira/browse/MESOS-324 > > > Diffs > ----- > > include/mesos/mesos.proto 38235157d45bdccb676e5c3241c21b585a6f8801 > src/examples/java/TestFramework.java > 8417394487a80b439e7d9897c83f0b2c1eb17ff4 > src/examples/long_lived_framework.cpp > 04ac678387dd78104b5d42fa1f7b5de1849b0701 > src/examples/python/test_framework.py > 269f532733ab82378bc1bd643107e6f9f2945d8b > src/examples/test_framework.cpp b9ab692414e4df64f176fc1ecd05f24ff089bde0 > src/master/http.cpp 790961d6890841c456485491516d1991fe35ec16 > src/slave/http.cpp b94197013ac0d5dd95d6dabb44812905a123184a > src/slave/slave.hpp e9f7b659ca2860501840b3d01e69915ebd162039 > src/webui/master/static/slave_executor.html > bd06e69b3c99a0a44993c31429d877914b644a1d > src/webui/master/static/slave_framework.html > c66150ecb794300239c92ceaf684bfd3f1cb007e > > Diff: https://reviews.apache.org/r/9089/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Ben Mahler > >
