Re: Review Request 64590: Stopped logging optional fields unconditionally in agent handler.

2018-01-02 Thread Greg Mann 

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




src/slave/slave.cpp
Lines 7294 (patched)


Since this field name has been changed, we should update this log. But we 
still want to be sure it's clear that it's the operation's UUID we're logging, 
not the status's.

Maybe s/operation_uuid/operation uuid/ ?


- Greg Mann


On Dec. 28, 2017, 8:21 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64590/
> ---
> 
> (Updated Dec. 28, 2017, 8:21 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `operation_id` and `framework_id` fields are optional, so they
> should only be logged if set.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp e0806ab15be3ea790681dc97b20791c8a20512e3 
> 
> 
> Diff: https://reviews.apache.org/r/64590/diff/5/
> 
> 
> Testing
> ---
> 
> `make check` on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 64590: Stopped logging optional fields unconditionally in agent handler.

2017-12-28 Thread Gaston Kleiman 

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

(Updated Dec. 28, 2017, 12:21 p.m.)


Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Jie Yu.


Changes
---

Addressed feedback, rebased.


Repository: mesos


Description
---

The `operation_id` and `framework_id` fields are optional, so they
should only be logged if set.


Diffs (updated)
-

  src/slave/slave.cpp e0806ab15be3ea790681dc97b20791c8a20512e3 


Diff: https://reviews.apache.org/r/64590/diff/5/

Changes: https://reviews.apache.org/r/64590/diff/4-5/


Testing
---

`make check` on GNU/Linux


Thanks,

Gaston Kleiman

Re: Review Request 64590: Stopped logging optional fields unconditionally in agent handler.

2017-12-20 Thread Chun-Hung Hsiao 

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




src/slave/slave.cpp
Line 7428 (original), 7439 (patched)


Don't we need to check `operation->info().has_id()` here?


- Chun-Hung Hsiao


On Dec. 18, 2017, 11:24 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64590/
> ---
> 
> (Updated Dec. 18, 2017, 11:24 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `operation_id` and `framework_id` fields are optional, so they
> should only be logged if set.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp e69d42a2bfae09e2defcd8333c0434a9fd8bac4c 
> 
> 
> Diff: https://reviews.apache.org/r/64590/diff/4/
> 
> 
> Testing
> ---
> 
> `make check` on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 64590: Stopped logging optional fields unconditionally in agent handler.

2017-12-18 Thread Mesos Reviewbot Windows 

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



PASS: Mesos patch 64590 was successfully built and tested.

Reviews applied: `['64590']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64590

- Mesos Reviewbot Windows


On Dec. 18, 2017, 11:24 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64590/
> ---
> 
> (Updated Dec. 18, 2017, 11:24 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `operation_id` and `framework_id` fields are optional, so they
> should only be logged if set.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp e69d42a2bfae09e2defcd8333c0434a9fd8bac4c 
> 
> 
> Diff: https://reviews.apache.org/r/64590/diff/4/
> 
> 
> Testing
> ---
> 
> `make check` on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 64590: Stopped logging optional fields unconditionally in agent handler.

2017-12-18 Thread Gaston Kleiman 

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

(Updated Dec. 18, 2017, 3:24 p.m.)


Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Jie Yu.


Changes
---

Addressed feedback.


Repository: mesos


Description
---

The `operation_id` and `framework_id` fields are optional, so they
should only be logged if set.


Diffs (updated)
-

  src/slave/slave.cpp e69d42a2bfae09e2defcd8333c0434a9fd8bac4c 


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

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


Testing
---

`make check` on GNU/Linux


Thanks,

Gaston Kleiman

Re: Review Request 64590: Stopped logging optional fields unconditionally in agent handler.

2017-12-18 Thread Greg Mann 

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


Fix it, then Ship it!





src/slave/slave.cpp
Lines 7276 (patched)


Nit: could we use operation_uuid here instead? Otherwise it's ambiguous if 
this UUID is for the status update or the operation.


- Greg Mann


On Dec. 14, 2017, 6:22 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64590/
> ---
> 
> (Updated Dec. 14, 2017, 6:22 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `operation_id` and `framework_id` fields are optional, so they
> should only be logged if set.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 9d0e9debf2ecd142eb6179c12ef4a0ada24b1c6c 
> 
> 
> Diff: https://reviews.apache.org/r/64590/diff/3/
> 
> 
> Testing
> ---
> 
> `make check` on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 64590: Stopped logging optional fields unconditionally in agent handler.

2017-12-14 Thread Mesos Reviewbot Windows 

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



FAIL: Some Mesos tests failed.

Reviews applied: `['64590']`

Failed command: `D:\DCOS\mesos\src\mesos-tests.exe --verbose`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64590

Relevant logs:

- 
[mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64590/logs/mesos-tests-stdout.log):

```

[--] 1 test from IsolationFlag/CpuIsolatorTest
[ RUN  ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0
[   OK ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0 (2304 ms)
[--] 1 test from IsolationFlag/CpuIsolatorTest (2326 ms total)

[--] 1 test from IsolationFlag/MemoryIsolatorTest
[ RUN  ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0
[   OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (2239 ms)
[--] 1 test from IsolationFlag/MemoryIsolatorTest (2261 ms total)

[--] Global test environment tear-down
[==] 835 tests from 85 test cases ran. (308347 ms total)
[  PASSED  ] 825 tests.
[  FAILED  ] 10 tests, listed below:
[  FAILED  ] OfferOperationStatusUpdateManagerTest.UpdateAndAckNonTerminalUpdate
[  FAILED  ] OfferOperationStatusUpdateManagerTest.RecoverCheckpointedStream
[  FAILED  ] OfferOperationStatusUpdateManagerTest.RecoverEmptyFile
[  FAILED  ] OfferOperationStatusUpdateManagerTest.RecoverTerminatedStream
[  FAILED  ] OfferOperationStatusUpdateManagerTest.IgnoreDuplicateUpdate
[  FAILED  ] 
OfferOperationStatusUpdateManagerTest.IgnoreDuplicateUpdateAfterRecover
[  FAILED  ] OfferOperationStatusUpdateManagerTest.RejectDuplicateAck
[  FAILED  ] 
OfferOperationStatusUpdateManagerTest.RejectDuplicateAckAfterRecover
[  FAILED  ] 
OfferOperationStatusUpdateManagerTest.NonStrictRecoveryCorruptedFile
[  FAILED  ] OfferOperationStatusUpdateManagerTest.UpdateLatestWhenResending

10 FAILED TESTS
  YOU HAVE 205 DISABLED TESTS

```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64590/logs/mesos-tests-stderr.log):

```
I1215 00:07:20.418342  4992 slave.cpp:3401] Shutting down framework 
c8e4e0c9-cc00-404f-b6ec-a81d26b237d4-
I1215 00:07:20.418342  1716 master.cpp:10160] Updating the state of task 
c82b6c1c-acfd-4f82-a0da-3322d9ab5be1 of framework c8e4e0c9-cc00-40I1215 
00:07:19.761358  7208 exec.cpp:162] Version: 1.5.0
I1215 00:07:19.784348  4320 exec.cpp:237] Executor registered on agent 
c8e4e0c9-cc00-404f-b6ec-a81d26b237d4-S0
I1215 00:07:19.788349  7748 executor.cpp:171] Received SUBSCRIBED event
I1215 00:07:19.792330  7748 executor.cpp:175] Subscribed executor on 
build-srv-04.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I1215 00:07:19.792330  7748 executor.cpp:171] Received LAUNCH event
I1215 00:07:19.796350  7748 executor.cpp:638] Starting task 
c82b6c1c-acfd-4f82-a0da-3322d9ab5be1
I1215 00:07:19.869345  7748 executor.cpp:478] Running 
'D:\DCOS\mesos\src\mesos-containerizer.exe launch '
I1215 00:07:20.394320  7748 executor.cpp:651] Forked command at 8812
I1215 00:07:20.419319  6644 exec.cpp:435] Executor asked to shutdown
I1215 00:07:20.420320   880 executor.cpp:171] Received SHUTDOWN event
I1215 00:07:20.420320   880 executor.cpp:748] Shutting down
I1215 00:07:20.420320   880 executor.cpp:855] Sending SIGTERM to process tree 
at pid 84f-b6ec-a81d26b237d4- (latest state: TASK_KILLED, status update 
state: TASK_KILLED)
I1215 00:07:20.418342  4992 slave.cpp:6109] Shutting down executor 
'c82b6c1c-acfd-4f82-a0da-3322d9ab5be1' of framework 
c8e4e0c9-cc00-404f-b6ec-a81d26b237d4- at executor(1)@10.3.1.5:53598
I1215 00:07:20.419319  4992 slave.cpp:909] Agent terminating
W1215 00:07:20.419319  4992 slave.cpp:3397] Ignoring shutdown framework 
c8e4e0c9-cc00-404f-b6ec-a81d26b237d4- because it is terminating
I1215 00:07:20.421320  1716 master.cpp:10266] Removing task 
c82b6c1c-acfd-4f82-a0da-3322d9ab5be1 with resources cpus(allocated: *):4; 
mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
*):[31000-32000] of framework c8e4e0c9-cc00-404f-b6ec-a81d26b237d4- on 
agent c8e4e0c9-cc00-404f-b6ec-a81d26b237d4-S0 at slave(327)@10.3.1.5:53577 
(build-srv-04.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I1215 00:07:20.422320  6024 containerizer.cpp:2337] Destroying container 
9857200e-9511-49d4-bcde-ce1478b370fb in RUNNING state
I1215 00:07:20.422320  6024 containerizer.cpp:2939] Transitioning the state of 
container 9857200e-9511-49d4-bcde-ce1478b370fb from RUNNING to DESTROYING
I1215 00:07:20.423318  1716 master.cpp:1305] Agent 
c8e4e0c9-cc00-404f-b6ec-a81d26b237d4-S0 at slave(327)@10.3.1.5:53577 
(build-srv-04.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) disconnected
I1215 00:07:20.423318  1716 master.cpp:3364] Disconnecting agent

Re: Review Request 64590: Stopped logging optional fields unconditionally in agent handler.

2017-12-14 Thread Mesos Reviewbot Windows 

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



FAIL: Mesos tests failed to build.

Reviews applied: `['64590']`

Failed command: `cmake.exe --build . --target mesos-tests --config Debug`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64590

Relevant logs:

- 
[mesos-tests-build-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64590/logs/mesos-tests-build-cmake-stdout.log):

```
  D:\DCOS\mesos\mesos\src\authentication\cram_md5\authenticatee.cpp(271): 
warning C4267: 'argument': conversion from 'size_t' to 'unsigned int', possible 
loss of data [D:\DCOS\mesos\src\mesos.vcxproj]
  D:\DCOS\mesos\mesos\src\authentication\cram_md5\authenticatee.cpp(333): 
warning C4267: '=': conversion from 'size_t' to 'unsigned int', possible loss 
of data [D:\DCOS\mesos\src\mesos.vcxproj]
  D:\DCOS\mesos\mesos\src\authentication\cram_md5\authenticator.cpp(216): 
warning C4267: 'argument': conversion from 'size_t' to 'unsigned int', possible 
loss of data [D:\DCOS\mesos\src\mesos.vcxproj]
  D:\DCOS\mesos\mesos\src\authentication\cram_md5\authenticator.cpp(242): 
warning C4267: 'argument': conversion from 'size_t' to 'unsigned int', possible 
loss of data [D:\DCOS\mesos\src\mesos.vcxproj]
  D:\DCOS\mesos\mesos\src\authentication\cram_md5\authenticator.cpp(274): 
warning C4267: '=': conversion from 'size_t' to 'unsigned int', possible loss 
of data [D:\DCOS\mesos\src\mesos.vcxproj]
  D:\DCOS\mesos\mesos\src\master/master.hpp(2093): warning C4244: 'return': 
conversion from 'unsigned __int64' to 'double', possible loss of data 
(compiling source file D:\DCOS\mesos\mesos\src\common\protobuf_utils.cpp) 
[D:\DCOS\mesos\src\mesos.vcxproj]
  D:\DCOS\mesos\mesos\src\files\files.cpp(704): warning C4267: 'argument': 
conversion from 'size_t' to 'off_t', possible loss of data 
[D:\DCOS\mesos\src\mesos.vcxproj]
  D:\DCOS\mesos\mesos\src\master/master.hpp(2093): warning C4244: 'return': 
conversion from 'unsigned __int64' to 'double', possible loss of data 
(compiling source file D:\DCOS\mesos\mesos\src\master\master.cpp) 
[D:\DCOS\mesos\src\mesos.vcxproj]
  D:\DCOS\mesos\mesos\src\master/master.hpp(2093): warning C4244: 'return': 
conversion from 'unsigned __int64' to 'double', possible loss of data 
(compiling source file D:\DCOS\mesos\mesos\src\master\quota_handler.cpp) 
[D:\DCOS\mesos\src\mesos.vcxproj]
  D:\DCOS\mesos\mesos\src\master/master.hpp(2093): warning C4244: 'return': 
conversion from 'unsigned __int64' to 'double', possible loss of data 
(compiling source file D:\DCOS\mesos\mesos\src\master\weights_handler.cpp) 
[D:\DCOS\mesos\src\mesos.vcxproj]
  D:\DCOS\mesos\mesos\src\master\master.cpp(6223): warning C4244: 'argument': 
conversion from 'const ::size_t' to 'double', possible loss of data 
[D:\DCOS\mesos\src\mesos.vcxproj]
  D:\DCOS\mesos\mesos\src\master\master.cpp(6325): warning C4244: 'argument': 
conversion from 'const ::size_t' to 'double', possible loss of data 
[D:\DCOS\mesos\src\mesos.vcxproj]
  D:\DCOS\mesos\mesos\src\master\master.cpp(6890): warning C4244: 'argument': 
conversion from 'const ::size_t' to 'double', possible loss of data 
[D:\DCOS\mesos\src\mesos.vcxproj]
  D:\DCOS\mesos\mesos\src\master\allocator\sorter\drf\sorter.cpp(589): warning 
C4267: 'return': conversion from 'size_t' to 'int', possible loss of data 
[D:\DCOS\mesos\src\mesos.vcxproj]
  D:\DCOS\mesos\mesos\src\master\master.cpp(9014): warning C4244: 'argument': 
conversion from 'const ::size_t' to 'double', possible loss of data 
[D:\DCOS\mesos\src\mesos.vcxproj]
  D:\DCOS\mesos\mesos\src\master\master.cpp(10855): warning C4244: 'return': 
conversion from '::size_t' to 'double', possible loss of data 
[D:\DCOS\mesos\src\mesos.vcxproj]


"D:\DCOS\mesos\src\tests\mesos-tests.vcxproj" (default target) (1) ->
"D:\DCOS\mesos\src\mesos.vcxproj" (default target) (12) ->
(ClCompile target) -> 
  D:\DCOS\mesos\mesos\src\slave\slave.cpp(7278): error C2678: binary '<<': no 
operator found which takes a left-hand operand of type 'const char [16]' (or 
there is no acceptable conversion) [D:\DCOS\mesos\src\mesos.vcxproj]
  D:\DCOS\mesos\mesos\src\slave\slave.cpp(7293): error C2678: binary '<<': no 
operator found which takes a left-hand operand of type 'const char [16]' (or 
there is no acceptable conversion) [D:\DCOS\mesos\src\mesos.vcxproj]
  D:\DCOS\mesos\mesos\src\slave\slave.cpp(7284): error C2146: syntax error: 
missing ';' before identifier 'OfferOperationStatusUpdate' 
[D:\DCOS\mesos\src\mesos.vcxproj]

135 Warning(s)
3 Error(s)

Time Elapsed 00:03:05.73
```

- 
[mesos-tests-CMakeOutput.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64590/logs/mesos-tests-CMakeOutput.log):

```
  Creating directory "cmTC_1c679.dir\Debug\".

  Creating directory

Re: Review Request 64590: Stopped logging optional fields unconditionally in agent handler.

2017-12-14 Thread Gaston Kleiman 

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

(Updated Dec. 14, 2017, 10:22 a.m.)


Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Jie Yu.


Changes
---

Rebased & addressed feedback.


Summary (updated)
-

Stopped logging optional fields unconditionally in agent handler.


Repository: mesos


Description (updated)
---

The `operation_id` and `framework_id` fields are optional, so they
should only be logged if set.


Diffs (updated)
-

  src/slave/slave.cpp 9d0e9debf2ecd142eb6179c12ef4a0ada24b1c6c 


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

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


Testing
---

`make check` on GNU/Linux


Thanks,

Gaston Kleiman