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:40 UTC

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

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());