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 2015/10/20 22:25:20 UTC
mesos git commit: Fixed typos in log messages and comments in
replicated log code.
Repository: mesos
Updated Branches:
refs/heads/master 5e68b23e7 -> 6da93b559
Fixed typos in log messages and comments in replicated log code.
Fixed typos in log messages and comments; minor code/style cleanup.
Mostly in the replicated log code.
Review: https://reviews.apache.org/r/39463
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/6da93b55
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/6da93b55
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/6da93b55
Branch: refs/heads/master
Commit: 6da93b5591ca764104630be57089e425ca26e9ab
Parents: 5e68b23
Author: Neil Conway <ne...@gmail.com>
Authored: Tue Oct 20 16:18:05 2015 -0400
Committer: Joris Van Remoortere <jo...@gmail.com>
Committed: Tue Oct 20 16:24:21 2015 -0400
----------------------------------------------------------------------
src/Makefile.am | 2 +-
src/log/consensus.cpp | 7 ++++---
src/log/coordinator.cpp | 2 +-
src/log/replica.cpp | 46 ++++++++++++++++++++----------------------
src/log/replica.hpp | 4 ++--
src/state/log.cpp | 2 +-
src/tests/log_tests.cpp | 42 ++++++++++++++++++++------------------
src/tests/slave_tests.cpp | 3 ++-
8 files changed, 55 insertions(+), 53 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/6da93b55/src/Makefile.am
----------------------------------------------------------------------
diff --git a/src/Makefile.am b/src/Makefile.am
index 0dc9112..98cbafc 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1168,7 +1168,7 @@ dist_bin_SCRIPTS += \
cli/mesos-tail
# Need to distribute/install webui javascript. We use 'pkgdatadir'
-# instead of 'datadir' as the install directory so we get the the
+# instead of 'datadir' as the install directory so we get the
# package name (i.e., 'mesos') as part of the path (i.e.,
# /path/to/mesos/webui versus something like /path/to/webui). Note
# that 'datadir' (e.g., /usr/local/share) is for read-only "data" and
http://git-wip-us.apache.org/repos/asf/mesos/blob/6da93b55/src/log/consensus.cpp
----------------------------------------------------------------------
diff --git a/src/log/consensus.cpp b/src/log/consensus.cpp
index 59f80d0..71cd5f3 100644
--- a/src/log/consensus.cpp
+++ b/src/log/consensus.cpp
@@ -162,6 +162,7 @@ private:
// might return the actual action at this position. Picking
// either action is _correct_, since eventually we know this
// position will be truncated. Fun!
+ // TODO(neilc): Create a test case for this scenario.
promise.set(response);
// The remaining responses will be discarded in 'finalize'.
@@ -182,9 +183,9 @@ private:
// promised to us. No need to do anything here.
}
} else {
- // Received a response without an action associated with. This
- // is the case where this proposer is this first one who asks
- // promise for this log position.
+ // Received a response without an action associated with it.
+ // This is the case when this proposer is the first to request
+ // a promise for this log position.
CHECK(response.has_position());
CHECK_EQ(response.position(), position);
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/6da93b55/src/log/coordinator.cpp
----------------------------------------------------------------------
diff --git a/src/log/coordinator.cpp b/src/log/coordinator.cpp
index 5500bca..e1df8b0 100644
--- a/src/log/coordinator.cpp
+++ b/src/log/coordinator.cpp
@@ -228,7 +228,7 @@ Future<IntervalSet<uint64_t> > CoordinatorProcess::getMissingPositions()
Future<Nothing> CoordinatorProcess::catchupMissingPositions(
const IntervalSet<uint64_t>& positions)
{
- LOG(INFO) << "Coordinator attemping to fill missing position";
+ LOG(INFO) << "Coordinator attempting to fill missing positions";
// Notice that here we use "proposal + 1" as the proposal number for
// fill operations in order to avoid unnecessary retries for those
http://git-wip-us.apache.org/repos/asf/mesos/blob/6da93b55/src/log/replica.cpp
----------------------------------------------------------------------
diff --git a/src/log/replica.cpp b/src/log/replica.cpp
index 75d39ff..414e116 100644
--- a/src/log/replica.cpp
+++ b/src/log/replica.cpp
@@ -76,7 +76,7 @@ public:
// Returns all the actions between the specified positions, unless
// those positions are invalid, in which case returns an error.
- Future<list<Action> > read(uint64_t from, uint64_t to);
+ Future<list<Action>> read(uint64_t from, uint64_t to);
// Returns true if the specified position is missing in the log
// (i.e., unlearned or holes).
@@ -92,14 +92,14 @@ public:
// Returns the last written position in the log.
uint64_t ending();
- // Returns the current status of the this replica.
+ // Returns the current status of this replica.
Metadata::Status status();
// Returns the highest implicit promise this replica has given.
uint64_t promised();
- // Updates the status of this replica. The update will persisted on
- // the disk. Returns true on success and false otherwise.
+ // Updates the status of this replica. The update will be persisted
+ // to storage. Returns true on success and false otherwise.
bool update(const Metadata::Status& status);
private:
@@ -116,14 +116,14 @@ private:
// Handles a message notifying of a learned action.
void learned(const UPID& from, const Action& action);
- // Helper routines that write a record corresponding to the
- // specified argument. Returns true on success and false otherwise.
+ // Persists the specified action to storage. Returns true on success
+ // and false otherwise.
bool persist(const Action& action);
- // Helper routines that update metadata corresponding to the
- // specified argument. The update will be persisted on the disk.
- // Returns true on success and false otherwise.
- bool update(uint64_t promised);
+ // Updates the highest promise this replica has given. The update
+ // will be persisted to storage. Returns true on success and false
+ // otherwise.
+ bool updatePromised(uint64_t promised);
// Helper routine to restore log (e.g., on restart).
void restore(const string& path);
@@ -206,18 +206,18 @@ Result<Action> ReplicaProcess::read(uint64_t position)
// TODO(benh): Make this function actually return a Try once we change
// the future semantics to not include failures.
-Future<list<Action> > ReplicaProcess::read(uint64_t from, uint64_t to)
+Future<list<Action>> ReplicaProcess::read(uint64_t from, uint64_t to)
{
if (to < from) {
- process::Promise<list<Action> > promise;
+ process::Promise<list<Action>> promise;
promise.fail("Bad read range (to < from)");
return promise.future();
} else if (from < begin) {
- process::Promise<list<Action> > promise;
+ process::Promise<list<Action>> promise;
promise.fail("Bad read range (truncated position)");
return promise.future();
} else if (end < to) {
- process::Promise<list<Action> > promise;
+ process::Promise<list<Action>> promise;
promise.fail("Bad read range (past end of log)");
return promise.future();
}
@@ -231,7 +231,7 @@ Future<list<Action> > ReplicaProcess::read(uint64_t from, uint64_t to)
Result<Action> result = read(position);
if (result.isError()) {
- process::Promise<list<Action> > promise;
+ process::Promise<list<Action>> promise;
promise.fail(result.error());
return promise.future();
} else if (result.isSome()) {
@@ -329,7 +329,7 @@ bool ReplicaProcess::update(const Metadata::Status& status)
}
-bool ReplicaProcess::update(uint64_t promised)
+bool ReplicaProcess::updatePromised(uint64_t promised)
{
Metadata metadata_;
metadata_.set_status(status());
@@ -487,7 +487,7 @@ void ReplicaProcess::promise(const UPID& from, const PromiseRequest& request)
response.set_proposal(promised());
reply(response);
} else {
- if (update(request.proposal())) {
+ if (updatePromised(request.proposal())) {
// Return the last position written.
PromiseResponse response;
response.set_okay(true);
@@ -576,6 +576,8 @@ void ReplicaProcess::write(const UPID& from, const WriteRequest& request)
// MESOS-1271 for details). In that case, we want to prevent
// this log entry from being overwritten.
//
+ // TODO(neilc): Create a test-case for this scenario.
+ //
// TODO(benh): If the value in the write request is the same
// as the learned value, consider sending back an ACK which
// might speed convergence.
@@ -765,7 +767,7 @@ Replica::~Replica()
}
-Future<list<Action> > Replica::read(uint64_t from, uint64_t to) const
+Future<list<Action>> Replica::read(uint64_t from, uint64_t to) const
{
return dispatch(process, &ReplicaProcess::read, from, to);
}
@@ -777,7 +779,7 @@ Future<bool> Replica::missing(uint64_t position) const
}
-Future<IntervalSet<uint64_t> > Replica::missing(
+Future<IntervalSet<uint64_t>> Replica::missing(
uint64_t from, uint64_t to) const
{
return dispatch(process, &ReplicaProcess::missing, from, to);
@@ -810,11 +812,7 @@ Future<uint64_t> Replica::promised() const
Future<bool> Replica::update(const Metadata::Status& status)
{
- // Need to disambiguate overloaded function.
- bool (ReplicaProcess::*update)(const Metadata::Status& status) =
- &ReplicaProcess::update;
-
- return dispatch(process, update, status);
+ return dispatch(process, &ReplicaProcess::update, status);
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/6da93b55/src/log/replica.hpp
----------------------------------------------------------------------
diff --git a/src/log/replica.hpp b/src/log/replica.hpp
index 33d3f1d..70f415f 100644
--- a/src/log/replica.hpp
+++ b/src/log/replica.hpp
@@ -64,7 +64,7 @@ public:
// Returns all the actions between the specified positions, unless
// those positions are invalid, in which case returns an error.
- process::Future<std::list<Action> > read(
+ process::Future<std::list<Action>> read(
uint64_t from,
uint64_t to) const;
@@ -75,7 +75,7 @@ public:
// Returns missing positions in the log (i.e., unlearned or holes)
// within the specified range [from, to]. We use interval set, a
// more compact representation of set, to store missing positions.
- process::Future<IntervalSet<uint64_t> > missing(
+ process::Future<IntervalSet<uint64_t>> missing(
uint64_t from,
uint64_t to) const;
http://git-wip-us.apache.org/repos/asf/mesos/blob/6da93b55/src/state/log.cpp
----------------------------------------------------------------------
diff --git a/src/state/log.cpp b/src/state/log.cpp
index a75a605..1a18808 100644
--- a/src/state/log.cpp
+++ b/src/state/log.cpp
@@ -276,7 +276,7 @@ Future<Nothing> LogStorageProcess::_start(
// that entry when we read it instead.
if (index.isSome()) {
// If we've started before (i.e., have an 'index' position) we
- // should also expect know the last 'truncated' position.
+ // should also expect to know the last 'truncated' position.
CHECK_SOME(truncated);
return reader.read(index.get(), position.get())
.then(defer(self(), &Self::apply, lambda::_1));
http://git-wip-us.apache.org/repos/asf/mesos/blob/6da93b55/src/tests/log_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/log_tests.cpp b/src/tests/log_tests.cpp
index f2dd47c..222a12e 100644
--- a/src/tests/log_tests.cpp
+++ b/src/tests/log_tests.cpp
@@ -948,14 +948,14 @@ TEST_F(CoordinatorTest, Demoted)
EXPECT_SOME_EQ(0u, electing.get());
}
- uint64_t position;
+ uint64_t position1;
{
Future<Option<uint64_t> > appending = coord1.append("hello world");
AWAIT_READY(appending);
ASSERT_SOME(appending.get());
- position = appending.get().get();
- EXPECT_EQ(1u, position);
+ position1 = appending.get().get();
+ EXPECT_EQ(1u, position1);
}
Shared<Network> network2(new Network(pids));
@@ -965,7 +965,7 @@ TEST_F(CoordinatorTest, Demoted)
{
Future<Option<uint64_t> > electing = coord2.elect();
AWAIT_READY(electing);
- EXPECT_SOME_EQ(position, electing.get());
+ EXPECT_SOME_EQ(position1, electing.get());
}
{
@@ -974,19 +974,21 @@ TEST_F(CoordinatorTest, Demoted)
EXPECT_NONE(appending.get());
}
+ uint64_t position2;
+
{
Future<Option<uint64_t> > appending = coord2.append("hello hello");
AWAIT_READY(appending);
ASSERT_SOME(appending.get());
- position = appending.get().get();
- EXPECT_EQ(2u, position);
+ position2 = appending.get().get();
+ EXPECT_EQ(2u, position2);
}
{
- Future<list<Action> > actions = replica2->read(position, position);
+ Future<list<Action> > actions = replica2->read(position2, position2);
AWAIT_READY(actions);
ASSERT_EQ(1u, actions.get().size());
- EXPECT_EQ(position, actions.get().front().position());
+ EXPECT_EQ(position2, actions.get().front().position());
ASSERT_TRUE(actions.get().front().has_type());
ASSERT_EQ(Action::APPEND, actions.get().front().type());
EXPECT_EQ("hello hello", actions.get().front().append().bytes());
@@ -1046,8 +1048,8 @@ TEST_F(CoordinatorTest, Fill)
Coordinator coord2(2, replica3, network2);
{
- // Note that the first election should fail because 'coord2' get's
- // it's proposal number from 'replica3' which is any empty log and
+ // Note that the first election should fail because 'coord2' gets
+ // its proposal number from 'replica3' which has an empty log and
// thus a second attempt will need to be made.
Future<Option<uint64_t> > electing = coord2.elect();
AWAIT_READY(electing);
@@ -1126,8 +1128,8 @@ TEST_F(CoordinatorTest, NotLearnedFill)
Coordinator coord2(2, replica3, network2);
{
- // Note that the first election should fail because 'coord2' get's
- // it's proposal number from 'replica3' which is any empty log and
+ // Note that the first election should fail because 'coord2' gets
+ // its proposal number from 'replica3' which has an empty log and
// thus a second attempt will need to be made.
Future<Option<uint64_t> > electing = coord2.elect();
AWAIT_READY(electing);
@@ -1248,8 +1250,8 @@ TEST_F(CoordinatorTest, MultipleAppendsNotLearnedFill)
Coordinator coord2(2, replica3, network2);
{
- // Note that the first election should fail because 'coord2' get's
- // it's proposal number from 'replica3' which is any empty log and
+ // Note that the first election should fail because 'coord2' gets
+ // its proposal number from 'replica3' which has an empty log and
// thus a second attempt will need to be made.
Future<Option<uint64_t> > electing = coord2.elect();
AWAIT_READY(electing);
@@ -1389,8 +1391,8 @@ TEST_F(CoordinatorTest, TruncateNotLearnedFill)
Coordinator coord2(2, replica3, network2);
{
- // Note that the first election should fail because 'coord2' get's
- // it's proposal number from 'replica3' which is any empty log and
+ // Note that the first election should fail because 'coord2' gets
+ // its proposal number from 'replica3' which has an empty log and
// thus a second attempt will need to be made.
Future<Option<uint64_t> > electing = coord2.elect();
AWAIT_READY(electing);
@@ -1474,8 +1476,8 @@ TEST_F(CoordinatorTest, TruncateLearnedFill)
Coordinator coord2(2, replica3, network2);
{
- // Note that the first election should fail because 'coord2' get's
- // it's proposal number from 'replica3' which is any empty log and
+ // Note that the first election should fail because 'coord2' gets
+ // its proposal number from 'replica3' which has an empty log and
// thus a second attempt will need to be made.
Future<Option<uint64_t> > electing = coord2.elect();
AWAIT_READY(electing);
@@ -1577,8 +1579,8 @@ TEST_F(RecoverTest, RacingCatchup)
Coordinator coord2(3, shared4, network2);
{
- // Note that the first election should fail because 'coord2' get's
- // it's proposal number from 'replica3' which is any empty log and
+ // Note that the first election should fail because 'coord2' gets
+ // its proposal number from 'replica3' which has an empty log and
// thus a second attempt will need to be made.
Future<Option<uint64_t> > electing = coord2.elect();
AWAIT_READY(electing);
http://git-wip-us.apache.org/repos/asf/mesos/blob/6da93b55/src/tests/slave_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp
index 10a4fa7..91dbdba 100644
--- a/src/tests/slave_tests.cpp
+++ b/src/tests/slave_tests.cpp
@@ -157,7 +157,8 @@ TEST_F(SlaveTest, ShutdownUnregisteredExecutor)
// Need flags for 'executor_registration_timeout'.
slave::Flags flags = CreateSlaveFlags();
- // Set the isolation flag so we know a MesoContainerizer will be created.
+ // Set the isolation flag so we know a MesosContainerizer will
+ // be created.
flags.isolation = "posix/cpu,posix/mem";
Fetcher fetcher;