You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by mp...@apache.org on 2016/09/09 04:40:02 UTC

[1/3] kudu git commit: rpc: print nicer errors when mixing up HTTP and KRPC

Repository: kudu
Updated Branches:
  refs/heads/master 202e91249 -> 30a8337f0


rpc: print nicer errors when mixing up HTTP and KRPC

Users often accidentally try to navigate to a Kudu server's RPC port in
their browser, or try to send an RPC to the HTTP port. This patch
improves the error messages that are reported as follows:

* Clients trying to speak RPC to the HTTP port now see an error like:

F0908 16:08:15.823751 21988 kudu-admin.cc:168] Check failed: _s.ok() Bad
  status: IO error: Could not locate the leader master: Client connection
  negotiation failed: client connection to 127.0.0.1:8051: received
  invalid RPC message which appears to be an HTTP response. Verify that
  you have specified a valid RPC port and not an HTTP port.

instead of

F0908 16:09:42.489558 22016 kudu-admin.cc:168] Check failed: _s.ok() Bad
  status: IO error: Could not locate the leader master: Client connection
  negotiation failed: client connection to 127.0.0.1:8051: Received
  invalid message of size 1213486160 which exceeds the
  rpc_max_message_size of 52428800 bytes

* Servers which receive an HTTP request to their RPC port now report an
  error like:

0908 16:06:22.975401 (+    68us) negotiation.cc:234] Negotiation
  complete: Invalid argument: Server connection negotiation failed: server
  connection from 127.0.0.1:56866: invalid negotation, appears to be an
  HTTP client on the RPC port

instead of:

0908 16:04:47.236063 (+    88us) negotiation.cc:234] Negotiation
  complete: Invalid argument: Server connection negotiation failed: server
  connection from 127.0.0.1:56832: Connection must begin with magic
  number: hrpc

Change-Id: I77ce8c8903ddf18592ba634a90d7efc4083d572a
Reviewed-on: http://gerrit.cloudera.org:8080/4338
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy <mp...@apache.org>


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

Branch: refs/heads/master
Commit: 45a970a490b72b12af7fa0596bac9b0d80604b5b
Parents: 202e912
Author: Todd Lipcon <to...@apache.org>
Authored: Thu Sep 8 16:08:25 2016 -0700
Committer: Mike Percy <mp...@apache.org>
Committed: Fri Sep 9 04:39:25 2016 +0000

----------------------------------------------------------------------
 src/kudu/rpc/blocking_ops.cc  | 12 +++++++++++-
 src/kudu/rpc/serialization.cc |  8 +++++++-
 2 files changed, 18 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/45a970a4/src/kudu/rpc/blocking_ops.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/blocking_ops.cc b/src/kudu/rpc/blocking_ops.cc
index 6add2e8..8c1de60 100644
--- a/src/kudu/rpc/blocking_ops.cc
+++ b/src/kudu/rpc/blocking_ops.cc
@@ -38,6 +38,8 @@ namespace rpc {
 
 using google::protobuf::MessageLite;
 
+const char kHTTPHeader[] = "HTTP";
+
 Status EnsureBlockingMode(const Socket* const sock) {
   bool is_nonblocking;
   RETURN_NOT_OK(sock->IsNonBlocking(&is_nonblocking));
@@ -96,9 +98,17 @@ Status ReceiveFramedMessageBlocking(Socket* sock, faststring* recv_buf,
   // Verify that the payload size isn't out of bounds.
   // This can happen because of network corruption, or a naughty client.
   if (PREDICT_FALSE(payload_len > FLAGS_rpc_max_message_size)) {
+    // A common user mistake is to try to speak the Kudu RPC protocol to an
+    // HTTP endpoint, or vice versa.
+    if (memcmp(recv_buf->data(), kHTTPHeader, arraysize(kHTTPHeader)) == 0) {
+      return Status::IOError(
+          "received invalid RPC message which appears to be an HTTP response. "
+          "Verify that you have specified a valid RPC port and not an HTTP port.");
+    }
+
     return Status::IOError(
         strings::Substitute(
-            "Received invalid message of size $0 which exceeds"
+            "received invalid message of size $0 which exceeds"
             " the rpc_max_message_size of $1 bytes",
             payload_len, FLAGS_rpc_max_message_size));
   }

http://git-wip-us.apache.org/repos/asf/kudu/blob/45a970a4/src/kudu/rpc/serialization.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/serialization.cc b/src/kudu/rpc/serialization.cc
index c4248cc..91f6ec8 100644
--- a/src/kudu/rpc/serialization.cc
+++ b/src/kudu/rpc/serialization.cc
@@ -167,7 +167,13 @@ Status ValidateConnHeader(const Slice& slice) {
 
   // validate actual magic
   if (!slice.starts_with(kMagicNumber)) {
-    return Status::InvalidArgument("Connection must begin with magic number", kMagicNumber);
+    if (slice.starts_with("GET ") ||
+        slice.starts_with("POST") ||
+        slice.starts_with("HEAD")) {
+      return Status::InvalidArgument("invalid negotation, appears to be an HTTP client on "
+                                     "the RPC port");
+    }
+    return Status::InvalidArgument("connection must begin with magic number", kMagicNumber);
   }
 
   const uint8_t *data = slice.data();


[3/3] kudu git commit: Fix flakiness in tablet_replacement-itest

Posted by mp...@apache.org.
Fix flakiness in tablet_replacement-itest

This test was flaky because we were trying to perform a config change
when it was possible that the leader had not yet committed an operation
in its term.

Without the fix, I looped it 100 times with --stress_cpu_threads=8 and
it failed twice due to this issue. With the fix, I looped it 400 times
with the stress threads and only saw one failure, which seemed like a
straight timeout due to the stress threads monopolizing the CPU.

Change-Id: I15b361179f5e20f848f5b346e202b9217a35b61d
Reviewed-on: http://gerrit.cloudera.org:8080/4342
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy <mp...@apache.org>


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

Branch: refs/heads/master
Commit: 30a8337f05a8b92faff9400298aae640b6ebbecb
Parents: 814b082
Author: Todd Lipcon <to...@apache.org>
Authored: Thu Sep 8 20:53:02 2016 -0700
Committer: Mike Percy <mp...@apache.org>
Committed: Fri Sep 9 04:39:34 2016 +0000

----------------------------------------------------------------------
 src/kudu/integration-tests/tablet_replacement-itest.cc | 3 +++
 1 file changed, 3 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/30a8337f/src/kudu/integration-tests/tablet_replacement-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/tablet_replacement-itest.cc b/src/kudu/integration-tests/tablet_replacement-itest.cc
index 6962762..105bdc3 100644
--- a/src/kudu/integration-tests/tablet_replacement-itest.cc
+++ b/src/kudu/integration-tests/tablet_replacement-itest.cc
@@ -80,6 +80,9 @@ TEST_F(TabletReplacementITest, TestMasterTombstoneEvictedReplica) {
   ASSERT_OK(itest::StartElection(leader_ts, tablet_id, timeout));
   ASSERT_OK(itest::WaitForServersToAgree(timeout, ts_map_, tablet_id, 1)); // Wait for NO_OP.
 
+  // Wait until it has committed its NO_OP, so that we can perform a config change.
+  ASSERT_OK(itest::WaitUntilCommittedOpIdIndexIs(1, leader_ts, tablet_id, timeout));
+
   // Remove a follower from the config.
   ASSERT_OK(itest::RemoveServer(leader_ts, tablet_id, follower_ts, boost::none, timeout));
 


[2/3] kudu git commit: ksck: add a note about spurious checksum mismatches

Posted by mp...@apache.org.
ksck: add a note about spurious checksum mismatches

Safe-time isn't properly respected on the server side, which means that
running ksck in 'checksum' mode against an active table generates
spurious checksum mismatches. This just adds a note to that effect so
that users aren't confused.

Change-Id: I85babafb8d264be88dc71c9c878b53dae9c73901
Reviewed-on: http://gerrit.cloudera.org:8080/4341
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy <mp...@apache.org>


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

Branch: refs/heads/master
Commit: 814b08237b3e215bcb435cc19f5fbe0fe626f484
Parents: 45a970a
Author: Todd Lipcon <to...@apache.org>
Authored: Thu Sep 8 19:50:01 2016 -0700
Committer: Mike Percy <mp...@apache.org>
Committed: Fri Sep 9 04:39:30 2016 +0000

----------------------------------------------------------------------
 src/kudu/tools/ksck.cc | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/814b0823/src/kudu/tools/ksck.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tools/ksck.cc b/src/kudu/tools/ksck.cc
index 7165280..270765d 100644
--- a/src/kudu/tools/ksck.cc
+++ b/src/kudu/tools/ksck.cc
@@ -536,7 +536,11 @@ Status Ksck::ChecksumData(const ChecksumOptions& opts) {
                     num_results, num_tablet_replicas);
 
   if (num_mismatches != 0) {
-    return Status::Corruption(Substitute("$0 checksum mismatches were detected", num_mismatches));
+    // TODO(KUDU-1020): remove the below note once safe time advancement is fully implemented.
+    return Status::Corruption(Substitute(
+        "$0 checksum mismatches were detected. "
+        "NOTE: if the table is actively being written to, this may generate spurious "
+        "checksum mismatches.", num_mismatches));
   }
   if (num_errors != 0) {
     return Status::Aborted(Substitute("$0 errors were detected", num_errors));