You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by jo...@apache.org on 2016/12/08 02:44:37 UTC

[1/6] mesos git commit: Windows: Fixed default isolators in Agent.

Repository: mesos
Updated Branches:
  refs/heads/master 0701fa596 -> 4c0e45329


Windows: Fixed default isolators in Agent.

This commit sets the default isolators on Windows to Windows-specific
values, rather than POSIX-specific values.  This is a convenience for
users on Windows (whom no longer need to specify
`--isolation=windows/cpu,filesystem/windows`) and will allow tests
to exercise the default set of Agent flags.

In particular, this commit will transition Windows builds of the agent
away from using the `posix/cpu`, `posix/mem`, and `filesystem/posix`
isolators by default, replacing them with `windows/cpu` and
`filesystem/windows` (sadly, there is not yet a memory isolator for
Windows).

Review: https://reviews.apache.org/r/54470/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/4c0e4532
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/4c0e4532
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/4c0e4532

Branch: refs/heads/master
Commit: 4c0e453296e3ac7c5eda48a98eb7ad570c303d0a
Parents: b5b1ead
Author: Alex Clemmer <cl...@gmail.com>
Authored: Wed Dec 7 17:21:01 2016 -0800
Committer: Joseph Wu <jo...@apache.org>
Committed: Wed Dec 7 18:44:17 2016 -0800

----------------------------------------------------------------------
 docs/windows.md                                 |  2 +-
 src/slave/containerizer/mesos/containerizer.cpp | 18 ++++++++++++++----
 src/slave/flags.cpp                             | 10 ++++++++--
 src/tests/mesos.cpp                             |  2 ++
 4 files changed, 25 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/4c0e4532/docs/windows.md
----------------------------------------------------------------------
diff --git a/docs/windows.md b/docs/windows.md
index 0301527..a7747a2 100644
--- a/docs/windows.md
+++ b/docs/windows.md
@@ -57,7 +57,7 @@ Following are the instructions for stock Windows 10 and Windows Server 2012 or n
     # The Windows agent exposes new isolators that must be used as with
     # the `--isolation` flag. To get started point the agent to a working
     # master, using eiher an IP address or zookeeper information.
-    $ mesos-agent.exe --master=<master> --work_dir=<work folder> --isolation=windows/cpu,filesystem/windows --launcher_dir=<repository>\build\src
+    $ mesos-agent.exe --master=<master> --work_dir=<work folder> --launcher_dir=<repository>\build\src
 
 
 ## Known Limitations

http://git-wip-us.apache.org/repos/asf/mesos/blob/4c0e4532/src/slave/containerizer/mesos/containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp
index 6d50755..990da8b 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -149,10 +149,15 @@ Try<MesosContainerizer*> MesosContainerizer::create(
 
   if (flags.isolation == "process") {
     LOG(WARNING) << "The 'process' isolation flag is deprecated, "
-                 << "please update your flags to"
-                 << " '--isolation=posix/cpu,posix/mem'.";
+                 << "please update your flags to "
+                 << "'--isolation=posix/cpu,posix/mem' (or "
+                 << "'--isolation=windows/cpu' if you are on Windows).";
 
+#ifndef __WINDOWS__
     flags_.isolation = "posix/cpu,posix/mem";
+#else
+    flags_.isolation = "windows/cpu";
+#endif // !__WINDOWS__
   } else if (flags.isolation == "cgroups") {
     LOG(WARNING) << "The 'cgroups' isolation flag is deprecated, "
                  << "please update your flags to"
@@ -164,11 +169,16 @@ Try<MesosContainerizer*> MesosContainerizer::create(
   // One and only one filesystem isolator is required. The filesystem
   // isolator is responsible for preparing the filesystems for
   // containers (e.g., prepare filesystem roots, volumes, etc.). If
-  // the user does not specify one, 'filesystem/posix' will be used.
+  // the user does not specify one, 'filesystem/posix' (or
+  // 'filesystem/windows' on Windows) will be used
   //
   // TODO(jieyu): Check that only one filesystem isolator is used.
   if (!strings::contains(flags_.isolation, "filesystem/")) {
+#ifndef __WINDOWS__
     flags_.isolation += ",filesystem/posix";
+#else
+    flags_.isolation += ",filesystem/windows";
+#endif // !__WINDOWS__
   }
 
   if (strings::contains(flags_.isolation, "posix/disk")) {
@@ -289,7 +299,7 @@ Try<MesosContainerizer*> MesosContainerizer::create(
 
 #if ENABLE_XFS_DISK_ISOLATOR
     {"disk/xfs", &XfsDiskIsolatorProcess::create},
-#endif
+#endif // ENABLE_XFS_DISK_ISOLATOR
 #else
     {"windows/cpu", &WindowsCpuIsolatorProcess::create},
 #endif // __WINDOWS__

http://git-wip-us.apache.org/repos/asf/mesos/blob/4c0e4532/src/slave/flags.cpp
----------------------------------------------------------------------
diff --git a/src/slave/flags.cpp b/src/slave/flags.cpp
index 67326ee..d66e9d4 100644
--- a/src/slave/flags.cpp
+++ b/src/slave/flags.cpp
@@ -96,14 +96,20 @@ mesos::internal::slave::Flags::Flags()
 
   add(&Flags::isolation,
       "isolation",
-      "Isolation mechanisms to use, e.g., `posix/cpu,posix/mem`, or\n"
+      "Isolation mechanisms to use, e.g., `posix/cpu,posix/mem` (or \n"
+      "`windows/cpu` if you are on Windows), or\n"
       "`cgroups/cpu,cgroups/mem`, or network/port_mapping\n"
       "(configure with flag: `--with-network-isolator` to enable),\n"
       "or `gpu/nvidia` for nvidia specific gpu isolation,\n"
       "or load an alternate isolator module using the `--modules`\n"
       "flag. Note that this flag is only relevant for the Mesos\n"
       "Containerizer.",
-      "posix/cpu,posix/mem");
+#ifndef __WINDOWS__
+      "posix/cpu,posix/mem"
+#else
+      "windows/cpu"
+#endif // !__WINDOWS__
+      );
 
   add(&Flags::launcher,
       "launcher",

http://git-wip-us.apache.org/repos/asf/mesos/blob/4c0e4532/src/tests/mesos.cpp
----------------------------------------------------------------------
diff --git a/src/tests/mesos.cpp b/src/tests/mesos.cpp
index e9bd2c6..73f17a7 100644
--- a/src/tests/mesos.cpp
+++ b/src/tests/mesos.cpp
@@ -569,6 +569,8 @@ slave::Flags ContainerizerTest<slave::MesosContainerizer>::CreateSlaveFlags()
   } else {
     flags.isolation = "posix/cpu,posix/mem";
   }
+#elif defined(__WINDOWS__)
+  flags.isolation = "windows/cpu";
 #else
   flags.isolation = "posix/cpu,posix/mem";
 #endif


[3/6] mesos git commit: Remove use of `typename` causing Windows build break.

Posted by jo...@apache.org.
Remove use of `typename` causing Windows build break.

The use of `typename` in C++ is used to disambiguate between static
members and dependent names that use the same symbol. For example, if
you have some `T::iterator` and `T` happens to have a static member
called iterator, it is necessary to add `typename T::iterator` to
indicate that you want an iterator of `T` rather than to refer to teh
static member `T::iterator`.

In some cases we employ `typename` to make our intention clearer, even
though it is not strictly speaking necessary. While normally a good
habit, in the specific cases we change in this review, it causes MSVC to
explode.

This commit will remove these uses.

Review: https://reviews.apache.org/r/53552/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/96e39e04
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/96e39e04
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/96e39e04

Branch: refs/heads/master
Commit: 96e39e04e9618272b8562e2475ce2a46cd770987
Parents: 0701fa5
Author: Alex Clemmer <cl...@gmail.com>
Authored: Wed Dec 7 11:07:14 2016 -0800
Committer: Joseph Wu <jo...@apache.org>
Committed: Wed Dec 7 18:44:17 2016 -0800

----------------------------------------------------------------------
 src/master/allocator/mesos/hierarchical.cpp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/96e39e04/src/master/allocator/mesos/hierarchical.cpp
----------------------------------------------------------------------
diff --git a/src/master/allocator/mesos/hierarchical.cpp b/src/master/allocator/mesos/hierarchical.cpp
index 33db3b8..a8cbc8d 100644
--- a/src/master/allocator/mesos/hierarchical.cpp
+++ b/src/master/allocator/mesos/hierarchical.cpp
@@ -466,7 +466,7 @@ void HierarchicalAllocatorProcess::addSlave(
   // leverage state and features such as the FrameworkSorter and OfferFilter.
   if (unavailability.isSome()) {
     slaves[slaveId].maintenance =
-      typename Slave::Maintenance(unavailability.get());
+      Slave::Maintenance(unavailability.get());
   }
 
   // If we have just a number of recovered agents, we cannot distinguish
@@ -857,7 +857,7 @@ void HierarchicalAllocatorProcess::updateUnavailability(
   // If we have a new unavailability.
   if (unavailability.isSome()) {
     slaves[slaveId].maintenance =
-      typename Slave::Maintenance(unavailability.get());
+      Slave::Maintenance(unavailability.get());
   }
 
   allocate(slaveId);
@@ -881,7 +881,7 @@ void HierarchicalAllocatorProcess::updateInverseOffer(
 
   // We use a reference by alias because we intend to modify the
   // `maintenance` and to improve readability.
-  typename Slave::Maintenance& maintenance = slaves[slaveId].maintenance.get();
+  Slave::Maintenance& maintenance = slaves[slaveId].maintenance.get();
 
   // Only handle inverse offers that we currently have outstanding. If it is not
   // currently outstanding this means it is old and can be safely ignored.
@@ -1732,7 +1732,7 @@ void HierarchicalAllocatorProcess::deallocate(
       if (slaves[slaveId].maintenance.isSome()) {
         // We use a reference by alias because we intend to modify the
         // `maintenance` and to improve readability.
-        typename Slave::Maintenance& maintenance =
+        Slave::Maintenance& maintenance =
           slaves[slaveId].maintenance.get();
 
         hashmap<string, Resources> allocation =


[6/6] mesos git commit: Windows: Fixed implicit cast warning in dirent.hpp.

Posted by jo...@apache.org.
Windows: Fixed implicit cast warning in dirent.hpp.

Review: https://reviews.apache.org/r/53710/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/55bcdc46
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/55bcdc46
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/55bcdc46

Branch: refs/heads/master
Commit: 55bcdc46d6a6cced72c87a73060e82e83993687e
Parents: fe67ca7
Author: Daniel Pravat <dp...@outlook.com>
Authored: Wed Dec 7 15:04:21 2016 -0800
Committer: Joseph Wu <jo...@apache.org>
Committed: Wed Dec 7 18:44:17 2016 -0800

----------------------------------------------------------------------
 3rdparty/stout/include/stout/internal/windows/dirent.hpp | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/55bcdc46/3rdparty/stout/include/stout/internal/windows/dirent.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/internal/windows/dirent.hpp b/3rdparty/stout/include/stout/internal/windows/dirent.hpp
index fe33210..7dbdb10 100644
--- a/3rdparty/stout/include/stout/internal/windows/dirent.hpp
+++ b/3rdparty/stout/include/stout/internal/windows/dirent.hpp
@@ -194,7 +194,8 @@ inline bool open_dir_stream(DIR* directory)
   //
   // [1] https://msdn.microsoft.com/en-us/library/windows/desktop/aa365740(v=vs.85).aspx
   strcpy(directory->curr.d_name, directory->fd.cFileName);
-  directory->curr.d_namlen = strlen(directory->curr.d_name);
+  directory->curr.d_namlen =
+    static_cast<unsigned short>(strlen(directory->curr.d_name));
 
   return true;
 }
@@ -214,7 +215,8 @@ inline bool reentrant_advance_dir_stream(DIR* directory)
   //
   // [1] https://msdn.microsoft.com/en-us/library/windows/desktop/aa365740(v=vs.85).aspx
   strcpy(directory->curr.d_name, directory->fd.cFileName);
-  directory->curr.d_namlen = strlen(directory->curr.d_name);
+  directory->curr.d_namlen =
+    static_cast<unsigned short>(strlen(directory->curr.d_name));
 
   return true;
 }


[5/6] mesos git commit: Windows: Added APR include path to libprocess configuration.

Posted by jo...@apache.org.
Windows: Added APR include path to libprocess configuration.

Partially addresses MESOS-3447, as APR is a dependency of the SVN
facilities of Stout.

On Unix builds, APR is expected to have been installed on the system
prior to building Mesos (usually by a package manager). Since Windows
does not have a package manager or a reasonble way of automatically
discovering where a package is installed (aside from the registry), our
CMake build system takes it upon itself to manage these system
dependencies.  This means that on Windows, we need to configure the
build to look for the APR headers in our custom-downloaded APR
repository.  Currently, though, we are not doing this, so when we'll
hit a compile-time error if we try to build (e.g.) `svn.hpp`.

This commit will introduce the APR include paths as part of the build
against Stout. Since Stout is a header-only library, it is (right now)
incumbent on whoever is bundling Stout up to manage the third-party
dependencies of Stout. In our current implementation, libprocess manages
the APR dependency for Stout, hence, we put this logic in libprocess.

Review: https://reviews.apache.org/r/54462/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/b5b1ead3
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/b5b1ead3
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/b5b1ead3

Branch: refs/heads/master
Commit: b5b1ead3a8c28b8b65f514fd7b030324a735a26d
Parents: 1648491
Author: Alex Clemmer <cl...@gmail.com>
Authored: Wed Dec 7 16:57:37 2016 -0800
Committer: Joseph Wu <jo...@apache.org>
Committed: Wed Dec 7 18:44:17 2016 -0800

----------------------------------------------------------------------
 3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake | 1 +
 1 file changed, 1 insertion(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/b5b1ead3/3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake b/3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake
index 786e47e..e9eabd0 100644
--- a/3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake
+++ b/3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake
@@ -62,6 +62,7 @@ set(NVML_INCLUDE_DIR        ${NVML_ROOT})
 set(PICOJSON_INCLUDE_DIR    ${PICOJSON_ROOT})
 
 if (WIN32)
+  set(APR_INCLUDE_DIR      ${LIBAPR_ROOT}/include ${LIBAPR_ROOT}-build)
   set(CURL_INCLUDE_DIR     ${CURL_ROOT}/include)
   set(GLOG_INCLUDE_DIR     ${GLOG_ROOT}/src/windows)
   set(PROTOBUF_INCLUDE_DIR ${PROTOBUF_ROOT}/src)


[2/6] mesos git commit: Changed registrar backend to `in_memory` by default in tests.

Posted by jo...@apache.org.
Changed registrar backend to `in_memory` by default in tests.

Currently, all instances of the Master in tests set the
`--registry` flag to the default value (`replicated_log`).
When the `replicated_log` value is set, Masters in tests will
back the registrar with the disk, specifically via levelDB.

Only a small subset of tests actually require the `replicated_log`;
these are tests which expect the master to persist data across
failovers.  A majority of tests can be run with an `in_memory`
registrar backend.  Changing the default to `in_memory` will
serve multiple purposes:
* It will speed up the test suite by ~10-15%.
* It will reduce the flakiness observed on the ASF CI.
  These machines sometimes run into disk contention, which causes
  registrar reads/write to time out.
* It will unblock a majority of tests from being run on Windows,
  which currently does not implement a persistent registrar backend.

This review supercedes and revives: https://reviews.apache.org/r/41665/

Review: https://reviews.apache.org/r/54453/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/1648491e
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/1648491e
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/1648491e

Branch: refs/heads/master
Commit: 1648491e2f194f5ba9d62cb1e099066fb7f16272
Parents: 55bcdc4
Author: Alex Clemmer <cl...@gmail.com>
Authored: Wed Dec 7 16:13:22 2016 -0800
Committer: Joseph Wu <jo...@apache.org>
Committed: Wed Dec 7 18:44:17 2016 -0800

----------------------------------------------------------------------
 src/tests/dynamic_weights_tests.cpp |  2 ++
 src/tests/master_tests.cpp          | 15 +++++++++++++--
 src/tests/mesos.cpp                 |  6 ++++--
 src/tests/partition_tests.cpp       |  1 +
 src/tests/reconciliation_tests.cpp  |  9 +++++++--
 5 files changed, 27 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/1648491e/src/tests/dynamic_weights_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/dynamic_weights_tests.cpp b/src/tests/dynamic_weights_tests.cpp
index 6f1e249..ce577ce 100644
--- a/src/tests/dynamic_weights_tests.cpp
+++ b/src/tests/dynamic_weights_tests.cpp
@@ -617,6 +617,8 @@ TEST_F(DynamicWeightsTest, RecoveredWeightsFromRegistry)
 {
   // Start a master with `--weights` flag.
   master::Flags masterFlags = CreateMasterFlags();
+  masterFlags.registry = "replicated_log";
+
   masterFlags.weights = UPDATED_WEIGHTS1;
   Try<Owned<cluster::Master>> master = StartMaster(masterFlags);
   ASSERT_SOME(master);

http://git-wip-us.apache.org/repos/asf/mesos/blob/1648491e/src/tests/master_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_tests.cpp b/src/tests/master_tests.cpp
index de48594..7442eb8 100644
--- a/src/tests/master_tests.cpp
+++ b/src/tests/master_tests.cpp
@@ -477,7 +477,10 @@ TEST_F(MasterTest, KillUnknownTask)
 
 TEST_F(MasterTest, KillUnknownTaskSlaveInTransition)
 {
-  Try<Owned<cluster::Master>> master = StartMaster();
+  master::Flags masterFlags = CreateMasterFlags();
+  masterFlags.registry = "replicated_log";
+
+  Try<Owned<cluster::Master>> master = StartMaster(masterFlags);
   ASSERT_SOME(master);
 
   Future<SlaveRegisteredMessage> slaveRegisteredMessage =
@@ -546,7 +549,7 @@ TEST_F(MasterTest, KillUnknownTaskSlaveInTransition)
     .WillOnce(FutureSatisfy(&disconnected));
 
   // Restart master.
-  master = StartMaster();
+  master = StartMaster(masterFlags);
   ASSERT_SOME(master);
 
   // Intercept the first registrar operation that is attempted; this
@@ -2094,6 +2097,8 @@ TEST_F(MasterTest, RecoveredSlaveCanReregister)
 {
   // Step 1: Start a master.
   master::Flags masterFlags = CreateMasterFlags();
+  masterFlags.registry = "replicated_log";
+
   Try<Owned<cluster::Master>> master = StartMaster(masterFlags);
   ASSERT_SOME(master);
 
@@ -2183,6 +2188,8 @@ TEST_F(MasterTest, UnreachableTaskAfterFailover)
 {
   // Step 1: Start a master.
   master::Flags masterFlags = CreateMasterFlags();
+  masterFlags.registry = "replicated_log";
+
   Try<Owned<cluster::Master>> master = StartMaster(masterFlags);
   ASSERT_SOME(master);
 
@@ -2325,6 +2332,8 @@ TEST_F(MasterTest, RateLimitRecoveredSlaveRemoval)
 {
   // Start a master.
   master::Flags masterFlags = CreateMasterFlags();
+  masterFlags.registry = "replicated_log";
+
   Try<Owned<cluster::Master>> master = StartMaster(masterFlags);
   ASSERT_SOME(master);
 
@@ -2401,6 +2410,8 @@ TEST_F(MasterTest, CancelRecoveredSlaveRemoval)
 {
   // Start a master.
   master::Flags masterFlags = CreateMasterFlags();
+  masterFlags.registry = "replicated_log";
+
   Try<Owned<cluster::Master>> master = StartMaster(masterFlags);
   ASSERT_SOME(master);
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/1648491e/src/tests/mesos.cpp
----------------------------------------------------------------------
diff --git a/src/tests/mesos.cpp b/src/tests/mesos.cpp
index 8fd8bcb..e9bd2c6 100644
--- a/src/tests/mesos.cpp
+++ b/src/tests/mesos.cpp
@@ -143,8 +143,10 @@ master::Flags MesosTest::CreateMasterFlags()
   // Set default ACLs.
   flags.acls = ACLs();
 
-  // Use the replicated log (without ZooKeeper) by default.
-  flags.registry = "replicated_log";
+  // Use the in-memory registry (instead of the replicated log) by default.
+  // TODO(josephw): Consider changing this back to `replicated_log` once
+  // all platforms support this registrar backend.
+  flags.registry = "in_memory";
 
   // On many test VMs, this default is too small.
   flags.registry_store_timeout = flags.registry_store_timeout * 5;

http://git-wip-us.apache.org/repos/asf/mesos/blob/1648491e/src/tests/partition_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/partition_tests.cpp b/src/tests/partition_tests.cpp
index e823672..00cc815 100644
--- a/src/tests/partition_tests.cpp
+++ b/src/tests/partition_tests.cpp
@@ -1659,6 +1659,7 @@ TEST_F(PartitionTest, RegistryGcByCountManySlaves)
   // Configure GC to only keep the most recent partitioned agent in
   // the unreachable list.
   master::Flags masterFlags = CreateMasterFlags();
+  masterFlags.registry = "replicated_log";
   masterFlags.registry_max_agent_count = 1;
 
   Try<Owned<cluster::Master>> master = StartMaster(masterFlags);

http://git-wip-us.apache.org/repos/asf/mesos/blob/1648491e/src/tests/reconciliation_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/reconciliation_tests.cpp b/src/tests/reconciliation_tests.cpp
index 71073a0..aeeb109 100644
--- a/src/tests/reconciliation_tests.cpp
+++ b/src/tests/reconciliation_tests.cpp
@@ -445,7 +445,9 @@ TEST_F(ReconciliationTest, UnknownKillTask)
 TEST_F(ReconciliationTest, RecoveredAgent)
 {
   master::Flags masterFlags = CreateMasterFlags();
-  Try<Owned<cluster::Master>> master = StartMaster();
+  masterFlags.registry = "replicated_log";
+
+  Try<Owned<cluster::Master>> master = StartMaster(masterFlags);
   ASSERT_SOME(master);
 
   // Reuse slaveFlags so both StartSlave() use the same work_dir.
@@ -529,7 +531,9 @@ TEST_F(ReconciliationTest, RecoveredAgent)
 TEST_F(ReconciliationTest, RecoveredAgentReregistrationInProgress)
 {
   master::Flags masterFlags = CreateMasterFlags();
-  Try<Owned<cluster::Master>> master = StartMaster();
+  masterFlags.registry = "replicated_log";
+
+  Try<Owned<cluster::Master>> master = StartMaster(masterFlags);
   ASSERT_SOME(master);
 
   // Reuse slaveFlags so both StartSlave() use the same work_dir.
@@ -1134,6 +1138,7 @@ TEST_F(ReconciliationTest, ReconcileStatusUpdateTaskState)
 TEST_F(ReconciliationTest, PartitionedAgentThenMasterFailover)
 {
   master::Flags masterFlags = CreateMasterFlags();
+  masterFlags.registry = "replicated_log";
 
   Try<Owned<cluster::Master>> master = StartMaster(masterFlags);
   ASSERT_SOME(master);


[4/6] mesos git commit: Rename symbols in log.proto to avoid naming collision in win32 API.

Posted by jo...@apache.org.
Rename symbols in log.proto to avoid naming collision in win32 API.

The standard win32 headers define a macro, `IGNORE`, which presently
collides with two uses of the same symbol in log.proto. This causes a
compile error on Windows.

In this commit, we rename the symbol in the log.proto files. There are
two primary reasons for this.

The first is because the trick we have previously applied to get around
similar problems (in which we `#undef` the macro, and re-define as a
global constant) is made somewaht more complex by the fact that the
log.proto headers are generated by protocol buffers. To implement this
effectively, we'd have to individually `#undef` at every site we include
log.pb.h, which is not a huge deal given the number of #include sites,
but doesn't protect us against future build breaks.

The second is that the approach of re-naming the symbol in log.proto is
not very invasive: we need to change a handful of places where the
system is used, and we never have to think of the issue again. And,
because it is an internal API, we don't require a major version bump to
implement the change.

Review: https://reviews.apache.org/r/53550/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/fe67ca79
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/fe67ca79
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/fe67ca79

Branch: refs/heads/master
Commit: fe67ca79ba2fb1da64017226661c63ec8a9441d8
Parents: 96e39e0
Author: Alex Clemmer <cl...@gmail.com>
Authored: Wed Dec 7 11:08:35 2016 -0800
Committer: Joseph Wu <jo...@apache.org>
Committed: Wed Dec 7 18:44:17 2016 -0800

----------------------------------------------------------------------
 src/log/consensus.cpp   | 21 ++++++++++++---------
 src/log/coordinator.cpp |  2 +-
 src/log/replica.cpp     |  8 +++++---
 src/messages/log.proto  | 16 ++++++++++------
 src/tests/log_tests.cpp |  4 ++--
 5 files changed, 30 insertions(+), 21 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/fe67ca79/src/log/consensus.cpp
----------------------------------------------------------------------
diff --git a/src/log/consensus.cpp b/src/log/consensus.cpp
index 94ddf24..b83c61e 100644
--- a/src/log/consensus.cpp
+++ b/src/log/consensus.cpp
@@ -149,7 +149,8 @@ private:
 
   void received(const PromiseResponse& response)
   {
-    if (response.has_type() && response.type() == PromiseResponse::IGNORE) {
+    if (response.has_type() && response.type() ==
+        PromiseResponse::IGNORED) {
       ignoresReceived++;
 
       // A quorum of replicas have ignored the request.
@@ -157,10 +158,10 @@ private:
         LOG(INFO) << "Aborting explicit promise request because "
                   << ignoresReceived << " ignores received";
 
-        // If the "type" is PromiseResponse::IGNORE, the rest of the
+        // If the "type" is PromiseResponse::IGNORED, the rest of the
         // fields don't matter.
         PromiseResponse result;
-        result.set_type(PromiseResponse::IGNORE);
+        result.set_type(PromiseResponse::IGNORED);
 
         promise.set(result);
         terminate(self());
@@ -352,7 +353,8 @@ private:
 
   void received(const PromiseResponse& response)
   {
-    if (response.has_type() && response.type() == PromiseResponse::IGNORE) {
+    if (response.has_type() && response.type() ==
+        PromiseResponse::IGNORED) {
       ignoresReceived++;
 
       // A quorum of replicas have ignored the request.
@@ -360,10 +362,10 @@ private:
         LOG(INFO) << "Aborting implicit promise request because "
                   << ignoresReceived << " ignores received";
 
-        // If the "type" is PromiseResponse::IGNORE, the rest of the
+        // If the "type" is PromiseResponse::IGNORED, the rest of the
         // fields don't matter.
         PromiseResponse result;
-        result.set_type(PromiseResponse::IGNORE);
+        result.set_type(PromiseResponse::IGNORED);
 
         promise.set(result);
         terminate(self());
@@ -536,17 +538,18 @@ private:
   {
     CHECK_EQ(response.position(), request.position());
 
-    if (response.has_type() && response.type() == WriteResponse::IGNORE) {
+    if (response.has_type() && response.type() ==
+        WriteResponse::IGNORED) {
       ignoresReceived++;
 
       if (ignoresReceived >= quorum) {
         LOG(INFO) << "Aborting write request because "
                   << ignoresReceived << " ignores received";
 
-        // If the "type" is WriteResponse::IGNORE, the rest of the
+        // If the "type" is WriteResponse::IGNORED, the rest of the
         // fields don't matter.
         WriteResponse result;
-        result.set_type(WriteResponse::IGNORE);
+        result.set_type(WriteResponse::IGNORED);
 
         promise.set(result);
         terminate(self());

http://git-wip-us.apache.org/repos/asf/mesos/blob/fe67ca79/src/log/coordinator.cpp
----------------------------------------------------------------------
diff --git a/src/log/coordinator.cpp b/src/log/coordinator.cpp
index 72ef036..01d2179 100644
--- a/src/log/coordinator.cpp
+++ b/src/log/coordinator.cpp
@@ -195,7 +195,7 @@ Future<Option<uint64_t>> CoordinatorProcess::checkPromisePhase(
   CHECK(response.has_type());
 
   switch (response.type()) {
-  case PromiseResponse::IGNORE:
+  case PromiseResponse::IGNORED:
     // A quorum of replicas ignored the request, but it can be
     // retried.
     return None();

http://git-wip-us.apache.org/repos/asf/mesos/blob/fe67ca79/src/log/replica.cpp
----------------------------------------------------------------------
diff --git a/src/log/replica.cpp b/src/log/replica.cpp
index d596e61..108a412 100644
--- a/src/log/replica.cpp
+++ b/src/log/replica.cpp
@@ -33,7 +33,9 @@
 #include <stout/try.hpp>
 #include <stout/utils.hpp>
 
+#ifndef __WINDOWS__
 #include "log/leveldb.hpp"
+#endif // __WINDOWS__
 #include "log/replica.hpp"
 #include "log/storage.hpp"
 
@@ -354,7 +356,7 @@ bool ReplicaProcess::updatePromised(uint64_t promised)
 //     number, we return a REJECT response.
 //  2. If we can't vote on the request because we're in the wrong
 //     state (e.g., not finished the recovery or catchup protocols),
-//     we return an IGNORE response.
+//     we return an IGNORED response.
 //  3. If we encounter an error (e.g., I/O failure) handling the
 //     request, we log the error and silently ignore the request.
 //
@@ -377,7 +379,7 @@ void ReplicaProcess::promise(const UPID& from, const PromiseRequest& request)
               << " as it is in " << status() << " status";
 
     PromiseResponse response;
-    response.set_type(PromiseResponse::IGNORE);
+    response.set_type(PromiseResponse::IGNORED);
     response.set_okay(false);
     response.set_proposal(request.proposal());
     reply(response);
@@ -526,7 +528,7 @@ void ReplicaProcess::write(const UPID& from, const WriteRequest& request)
               << " as it is in " << status() << " status";
 
     WriteResponse response;
-    response.set_type(WriteResponse::IGNORE);
+    response.set_type(WriteResponse::IGNORED);
     response.set_okay(false);
     response.set_proposal(request.proposal());
     response.set_position(request.position());

http://git-wip-us.apache.org/repos/asf/mesos/blob/fe67ca79/src/messages/log.proto
----------------------------------------------------------------------
diff --git a/src/messages/log.proto b/src/messages/log.proto
index 6a2c26b..12c2d83 100644
--- a/src/messages/log.proto
+++ b/src/messages/log.proto
@@ -127,7 +127,7 @@ message PromiseRequest {
 // Represents a promise response corresponding to a promise request.
 // The kind of the response is given by the "type" field:
 //
-//   1. IGNORE: The recipient of the promise request was not in VOTING
+//   1. IGNORED: The recipient of the promise request was not in VOTING
 //      state, so it ignored the request.
 //   2. REJECT: The recipient of the proposal has already promised a
 //      proposer with a higher proposal number. This is called a
@@ -137,7 +137,7 @@ message PromiseRequest {
 // Before 0.26, we only sent responses for cases 2 and 3, so the
 // 'okay' field was used to distinguish these responses. For backward
 // compatibility, we continue setting 'okay' to false for both cases 1
-// and 2; this means old masters will treat IGNORE as a NACK: this
+// and 2; this means old masters will treat IGNORED as a NACK: this
 // might result in demoting the current coordinator, but that should
 // be tolerable. TODO(neilc): Remove 'okay' in 0.27.
 //
@@ -151,7 +151,9 @@ message PromiseResponse {
   enum Type {
     ACCEPT = 1;
     REJECT = 2;
-    IGNORE = 3;
+    // NOTE: Change in tense here is to avoid name collisions with
+    // Windows headers.
+    IGNORED = 3;
   }
 
   required bool okay = 1; // DEPRECATED
@@ -180,7 +182,7 @@ message WriteRequest {
 // Represents a write response corresponding to a write request. The
 // kind of the response is given by the "type" field:
 //
-//   1. IGNORE: The recipient of the write request was not in VOTING
+//   1. IGNORED: The recipient of the write request was not in VOTING
 //      state, so it ignored the request.
 //   2. REJECT: The recipient of the proposal has already promised a
 //      proposer with a higher proposal number. This is called a
@@ -190,7 +192,7 @@ message WriteRequest {
 // Before 0.26, we only sent responses for cases 2 and 3, so the
 // 'okay' field was used to distinguish these responses. For backward
 // compatibility, we continue setting 'okay' to false for both cases 1
-// and 2; this means old masters will treat IGNORE as a NACK: this
+// and 2; this means old masters will treat IGNORED as a NACK: this
 // might result in demoting the current coordinator, but that should
 // be tolerable. TODO(neilc): Remove 'okay' in 0.27.
 //
@@ -202,7 +204,9 @@ message WriteResponse {
   enum Type {
     ACCEPT = 1;
     REJECT = 2;
-    IGNORE = 3;
+    // NOTE: Change in tense here is to avoid name collisions with
+    // Windows headers.
+    IGNORED = 3;
   }
 
   required bool okay = 1; // DEPRECATED

http://git-wip-us.apache.org/repos/asf/mesos/blob/fe67ca79/src/tests/log_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/log_tests.cpp b/src/tests/log_tests.cpp
index 9995438..06efd1b 100644
--- a/src/tests/log_tests.cpp
+++ b/src/tests/log_tests.cpp
@@ -559,7 +559,7 @@ TEST_F(ReplicaTest, NonVoting)
 
   AWAIT_READY(promiseResponse_);
   PromiseResponse promiseResponse = promiseResponse_.get();
-  EXPECT_EQ(PromiseResponse::IGNORE, promiseResponse.type());
+  EXPECT_EQ(PromiseResponse::IGNORED, promiseResponse.type());
   EXPECT_FALSE(promiseResponse.okay());
   EXPECT_EQ(2u, promiseResponse.proposal());
 
@@ -574,7 +574,7 @@ TEST_F(ReplicaTest, NonVoting)
 
   AWAIT_READY(writeResponse_);
   WriteResponse writeResponse = writeResponse_.get();
-  EXPECT_EQ(WriteResponse::IGNORE, writeResponse.type());
+  EXPECT_EQ(WriteResponse::IGNORED, writeResponse.type());
   EXPECT_FALSE(writeResponse.okay());
   EXPECT_EQ(3u, writeResponse.proposal());
   EXPECT_EQ(1u, writeResponse.position());