You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by al...@apache.org on 2019/05/11 03:25:45 UTC
[kudu] branch master updated (252b696 -> 762f0fc)
This is an automated email from the ASF dual-hosted git repository.
alexey pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git.
from 252b696 KUDU-2775: Deflake DefaultSourceTest repartition tests
new 58f189d KUDU-2711 pt 4. Java support for GetTableLocations optimizations
new 2acfd25 [build] Fix Gradle Checkstyle tasks
new 762f0fc [util] introduce Synchronizer::WaitUntil()
The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../checkstyle/checkstyle.xml} | 91 ++++++++++++++++------
.../checkstyle/suppressions.xml} | 0
java/gradle/quality.gradle | 6 +-
.../org/apache/kudu/client/AsyncKuduClient.java | 48 ++++++++++--
.../kudu/client/GetTableLocationsRequest.java | 1 +
.../java/org/apache/kudu/client/LocatedTablet.java | 2 +
.../java/org/apache/kudu/client/RemoteTablet.java | 14 ++--
.../apache/kudu/client/TestAsyncKuduClient.java | 6 +-
.../org/apache/kudu/client/TestRemoteTablet.java | 14 ++--
src/kudu/master/hms_notification_log_listener.cc | 2 +-
src/kudu/util/async_util-test.cc | 90 +++++++++++++++------
src/kudu/util/async_util.h | 9 ++-
12 files changed, 206 insertions(+), 77 deletions(-)
rename java/{kudu_style.xml => config/checkstyle/checkstyle.xml} (72%)
rename java/{checkstyle_suppressions.xml => config/checkstyle/suppressions.xml} (100%)
[kudu] 02/03: [build] Fix Gradle Checkstyle tasks
Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 2acfd25b849a40e30654178b312d5d0bf720ffca
Author: Grant Henke <gr...@apache.org>
AuthorDate: Thu May 9 14:15:19 2019 -0500
[build] Fix Gradle Checkstyle tasks
Checkstyle was broken in a recent upgrade due to
backwards incompatible changes to Checkstyle rules.
This patch adjust the Checkstyle task to be a bit more
Gradle idiomatic and also updates the checkstyle rules
to be more current.
Change-Id: I8f4d3aac746240949a32c798e27cd708a77966a4
Reviewed-on: http://gerrit.cloudera.org:8080/13297
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
---
.../checkstyle/checkstyle.xml} | 91 ++++++++++++++++------
.../checkstyle/suppressions.xml} | 0
java/gradle/quality.gradle | 6 +-
3 files changed, 68 insertions(+), 29 deletions(-)
diff --git a/java/kudu_style.xml b/java/config/checkstyle/checkstyle.xml
similarity index 72%
rename from java/kudu_style.xml
rename to java/config/checkstyle/checkstyle.xml
index d0b8d6c..562955d 100644
--- a/java/kudu_style.xml
+++ b/java/config/checkstyle/checkstyle.xml
@@ -19,10 +19,10 @@
// under the License.
-->
<!DOCTYPE module PUBLIC
- "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
- "http://www.puppycrawl.com/dtds/configuration_1_3.dtd">
+ "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
+ "https://checkstyle.org/dtds/configuration_1_3.dtd">
-<!-- Forked from https://raw.githubusercontent.com/checkstyle/checkstyle/checkstyle-6.19/src/main/resources/google_checks.xml -->
+<!-- Forked from https://raw.githubusercontent.com/checkstyle/checkstyle/checkstyle-8.20/src/main/resources/google_checks.xml -->
<!--
Checkstyle configuration that checks the Google coding conventions from Google Java Style
@@ -42,25 +42,31 @@
<property name="severity" value="warning"/>
<property name="fileExtensions" value="java, properties, xml"/>
+ <!-- Excludes all 'module-info.java' files -->
+ <!-- See https://checkstyle.org/config_filefilters.html -->
+ <module name="BeforeExecutionExclusionFileFilter">
+ <property name="fileNamePattern" value="module\-info\.java$"/>
+ </module>
<!-- Checks for whitespace -->
<!-- See http://checkstyle.sf.net/config_whitespace.html -->
<module name="FileTabCharacter">
<property name="eachLine" value="true"/>
</module>
<!-- Turn off/on checkstyle with CHECKSTYLE:OFF CHECKSTYLE:ON comments -->
- <module name="SuppressionCommentFilter"/>
+ <module name="SuppressWithPlainTextCommentFilter"/>
<!-- Suppress generated sources -->
<module name="SuppressionFilter">
- <property name="file" value="${checkstyle.suppressions.file}"/>
+ <property name="file" value="${config_loc}/suppressions.xml"/>
<property name="optional" value="false"/>
</module>
<module name="TreeWalker">
- <module name="FileContentsHolder"/>
<module name="OuterTypeFilename"/>
<module name="IllegalTokenText">
<property name="tokens" value="STRING_LITERAL, CHAR_LITERAL"/>
- <property name="format" value="\\u00(08|09|0(a|A)|0(c|C)|0(d|D)|22|27|5(C|c))|\\(0(10|11|12|14|15|42|47)|134)"/>
- <property name="message" value="Avoid using corresponding octal or Unicode escape."/>
+ <property name="format"
+ value="\\u00(09|0(a|A)|0(c|C)|0(d|D)|22|27|5(C|c))|\\(0(10|11|12|14|15|42|47)|134)"/>
+ <property name="message"
+ value="Consider using special escape sequence instead of octal value or Unicode escaped value."/>
</module>
<module name="AvoidEscapedUnicodeCharacters">
<property name="allowEscapesForControlCharacters" value="true"/>
@@ -76,19 +82,27 @@
<module name="NoLineWrap"/>
<module name="EmptyBlock">
<property name="option" value="TEXT"/>
- <property name="tokens" value="LITERAL_TRY, LITERAL_FINALLY, LITERAL_IF, LITERAL_ELSE, LITERAL_SWITCH"/>
+ <property name="tokens"
+ value="LITERAL_TRY, LITERAL_FINALLY, LITERAL_IF, LITERAL_ELSE, LITERAL_SWITCH"/>
</module>
<module name="NeedBraces"/>
- <module name="LeftCurly">
- <property name="maxLineLength" value="100"/>
+ <module name="LeftCurly"/>
+ <module name="RightCurly">
+ <property name="id" value="RightCurlySame"/>
+ <property name="tokens"
+ value="LITERAL_TRY, LITERAL_CATCH, LITERAL_FINALLY, LITERAL_IF, LITERAL_ELSE,
+ LITERAL_DO"/>
</module>
- <module name="RightCurly"/>
<module name="RightCurly">
+ <property name="id" value="RightCurlyAlone"/>
<property name="option" value="alone"/>
- <property name="tokens" value="CLASS_DEF, METHOD_DEF, CTOR_DEF, LITERAL_FOR, LITERAL_WHILE, LITERAL_DO, STATIC_INIT, INSTANCE_INIT"/>
+ <property name="tokens"
+ value="CLASS_DEF, METHOD_DEF, CTOR_DEF, LITERAL_FOR, LITERAL_WHILE, STATIC_INIT,
+ INSTANCE_INIT"/>
</module>
<module name="WhitespaceAround">
<property name="allowEmptyConstructors" value="true"/>
+ <property name="allowEmptyLambdas" value="true"/>
<property name="allowEmptyMethods" value="true"/>
<property name="allowEmptyTypes" value="true"/>
<property name="allowEmptyLoops" value="true"/>
@@ -108,13 +122,32 @@
<property name="allowNoEmptyLineBetweenFields" value="true"/>
</module>
<module name="SeparatorWrap">
+ <property name="id" value="SeparatorWrapDot"/>
<property name="tokens" value="DOT"/>
<property name="option" value="nl"/>
</module>
<module name="SeparatorWrap">
+ <property name="id" value="SeparatorWrapComma"/>
<property name="tokens" value="COMMA"/>
<property name="option" value="EOL"/>
</module>
+ <module name="SeparatorWrap">
+ <!-- ELLIPSIS is EOL until https://github.com/google/styleguide/issues/258 -->
+ <property name="id" value="SeparatorWrapEllipsis"/>
+ <property name="tokens" value="ELLIPSIS"/>
+ <property name="option" value="EOL"/>
+ </module>
+ <module name="SeparatorWrap">
+ <!-- ARRAY_DECLARATOR is EOL until https://github.com/google/styleguide/issues/259 -->
+ <property name="id" value="SeparatorWrapArrayDeclarator"/>
+ <property name="tokens" value="ARRAY_DECLARATOR"/>
+ <property name="option" value="EOL"/>
+ </module>
+ <module name="SeparatorWrap">
+ <property name="id" value="SeparatorWrapMethodRef"/>
+ <property name="tokens" value="METHOD_REF"/>
+ <property name="option" value="nl"/>
+ </module>
<module name="PackageName">
<property name="format" value="^[a-z]+(\.[a-z][a-z0-9]*)*$"/>
<message key="name.invalidPattern"
@@ -125,23 +158,28 @@
value="Type name ''{0}'' must match pattern ''{1}''."/>
</module>
<module name="MemberName">
- <property name="format" value="^[a-z][a-zA-Z0-9]*$"/>
+ <property name="format" value="^[a-z][a-z0-9][a-zA-Z0-9]*$"/>
<message key="name.invalidPattern"
value="Member name ''{0}'' must match pattern ''{1}''."/>
</module>
<module name="ParameterName">
- <property name="format" value="^[a-z][a-zA-Z0-9]*$"/>
+ <property name="format" value="^[a-z]([a-z0-9][a-zA-Z0-9]*)?$"/>
<message key="name.invalidPattern"
value="Parameter name ''{0}'' must match pattern ''{1}''."/>
</module>
+ <module name="LambdaParameterName">
+ <property name="format" value="^[a-z]([a-z0-9][a-zA-Z0-9]*)?$"/>
+ <message key="name.invalidPattern"
+ value="Lambda parameter name ''{0}'' must match pattern ''{1}''."/>
+ </module>
<module name="CatchParameterName">
- <property name="format" value="^[a-z][a-zA-Z0-9]*$"/>
+ <property name="format" value="^[a-z]([a-z0-9][a-zA-Z0-9]*)?$"/>
<message key="name.invalidPattern"
value="Catch parameter name ''{0}'' must match pattern ''{1}''."/>
</module>
<module name="LocalVariableName">
<property name="tokens" value="VARIABLE_DEF"/>
- <property name="format" value="^[a-z][a-zA-Z0-9]*$"/>
+ <property name="format" value="^[a-z]([a-z0-9][a-zA-Z0-9]*)?$"/>
<property name="allowOneCharVarInForLoop" value="true"/>
<message key="name.invalidPattern"
value="Local variable name ''{0}'' must match pattern ''{1}''."/>
@@ -183,28 +221,32 @@
<!-- This is pedantic... -->
<!--
<module name="AbbreviationAsWordInName">
- <property name="ignoreFinal" value="true"/>
- <property name="allowedAbbreviationLength" value="2"/>
+ <property name="ignoreFinal" value="false"/>
+ <property name="allowedAbbreviationLength" value="1"/>
</module>
-->
<module name="OverloadMethodsDeclarationOrder"/>
<module name="VariableDeclarationUsageDistance"/>
<module name="CustomImportOrder">
- <property name="specialImportsRegExp" value="com.google"/>
<property name="sortImportsInGroupAlphabetically" value="true"/>
<!-- Kudu differs from Google import order to mirror the C++ include order -->
- <property name="customImportOrderRules" value="STATIC###STANDARD_JAVA_PACKAGE###THIRD_PARTY_PACKAGE###SAME_PACKAGE(3)"/>
+ <property name="customImportOrderRules"
+ value="STATIC###STANDARD_JAVA_PACKAGE###THIRD_PARTY_PACKAGE###SAME_PACKAGE(3)"/>
</module>
<module name="MethodParamPad"/>
<!-- Kudu style puts operators like + at end of line before continuation -->
<module name="OperatorWrap">
<property name="option" value="EOL"/>
- <property name="tokens" value="BAND, BOR, BSR, BXOR, DIV, EQUAL, GE, GT, LAND, LE, LITERAL_INSTANCEOF, LOR, LT, MINUS, MOD, NOT_EQUAL, PLUS, QUESTION, SL, SR, STAR "/>
+ <property name="tokens" value="BAND, BOR, BSR, BXOR, DIV, EQUAL, GE, GT, LAND, LE,
+ LITERAL_INSTANCEOF, LOR, LT, MINUS, MOD, NOT_EQUAL, PLUS, QUESTION, SL, SR, STAR "/>
</module>
<module name="AnnotationLocation">
- <property name="tokens" value="CLASS_DEF, INTERFACE_DEF, ENUM_DEF, METHOD_DEF, CTOR_DEF"/>
+ <property name="id" value="AnnotationLocationMostCases"/>
+ <property name="tokens"
+ value="CLASS_DEF, INTERFACE_DEF, ENUM_DEF, METHOD_DEF, CTOR_DEF"/>
</module>
<module name="AnnotationLocation">
+ <property name="id" value="AnnotationLocationVariables"/>
<property name="tokens" value="VARIABLE_DEF"/>
<property name="allowSamelineMultipleAnnotations" value="true"/>
</module>
@@ -213,7 +255,8 @@
<module name="NonEmptyAtclauseDescription"/>
<module name="JavadocTagContinuationIndentation"/>
<module name="SummaryJavadoc">
- <property name="forbiddenSummaryFragments" value="^@return the *|^This method returns |^A [{]@code [a-zA-Z0-9]+[}]( is a )"/>
+ <property name="forbiddenSummaryFragments"
+ value="^@return the *|^This method returns |^A [{]@code [a-zA-Z0-9]+[}]( is a )"/>
</module>
<module name="JavadocParagraph"/>
<module name="AtclauseOrder">
diff --git a/java/checkstyle_suppressions.xml b/java/config/checkstyle/suppressions.xml
similarity index 100%
rename from java/checkstyle_suppressions.xml
rename to java/config/checkstyle/suppressions.xml
diff --git a/java/gradle/quality.gradle b/java/gradle/quality.gradle
index c92d0b2..8bdc21a 100644
--- a/java/gradle/quality.gradle
+++ b/java/gradle/quality.gradle
@@ -25,12 +25,8 @@ apply plugin: "ru.vyarus.animalsniffer" // Ensures Java code uses APIs from a pa
apply plugin: "scalafmt" // Automatically formats Scala code on each build.
apply plugin: "net.ltgt.errorprone" // Performs static code analysis to look for bugs in Java code.
-
checkstyle {
- configFile = file("$rootDir/kudu_style.xml")
- configProperties = [
- "checkstyle.suppressions.file" : "$rootDir/checkstyle_suppressions.xml"
- ]
+ configDir = file("$rootProject.projectDir/config/checkstyle")
ignoreFailures = true
showViolations = true
}
[kudu] 03/03: [util] introduce Synchronizer::WaitUntil()
Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 762f0fcc30803c20158cc1a75f6eac2fa530a44f
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Mon May 6 19:10:39 2019 -0700
[util] introduce Synchronizer::WaitUntil()
Added Synchronizer::WaitUntil() method to avoid converting MonoTime
into MonoDelta when the deadline is specified as MonoTime.
Updated corresponding tests for Synchronizer as well.
Change-Id: I00586ac1ba49494ff08abae0d452ab9286a3e56f
Reviewed-on: http://gerrit.cloudera.org:8080/13255
Reviewed-by: Andrew Wong <aw...@cloudera.com>
Tested-by: Alexey Serbin <as...@cloudera.com>
---
src/kudu/master/hms_notification_log_listener.cc | 2 +-
src/kudu/util/async_util-test.cc | 90 +++++++++++++++++-------
src/kudu/util/async_util.h | 9 ++-
3 files changed, 75 insertions(+), 26 deletions(-)
diff --git a/src/kudu/master/hms_notification_log_listener.cc b/src/kudu/master/hms_notification_log_listener.cc
index b34d233..2237a4b 100644
--- a/src/kudu/master/hms_notification_log_listener.cc
+++ b/src/kudu/master/hms_notification_log_listener.cc
@@ -118,7 +118,7 @@ Status HmsNotificationLogListenerTask::WaitForCatchUp(const MonoTime& deadline)
wake_up_cv_.Signal();
}
- RETURN_NOT_OK_PREPEND(synchronizer.WaitFor(deadline - MonoTime::Now()),
+ RETURN_NOT_OK_PREPEND(synchronizer.WaitUntil(deadline),
"failed to wait for Hive Metastore notification log listener to catch up");
return Status::OK();
}
diff --git a/src/kudu/util/async_util-test.cc b/src/kudu/util/async_util-test.cc
index 5cb7a63..91f2baa 100644
--- a/src/kudu/util/async_util-test.cc
+++ b/src/kudu/util/async_util-test.cc
@@ -28,6 +28,7 @@
#include "kudu/gutil/basictypes.h"
#include "kudu/gutil/callback.h"
#include "kudu/util/monotime.h"
+#include "kudu/util/scoped_cleanup.h"
#include "kudu/util/status.h"
#include "kudu/util/test_macros.h"
#include "kudu/util/test_util.h"
@@ -43,7 +44,7 @@ class AsyncUtilTest : public KuduTest {
// Set up an alarm to fail the test in case of deadlock.
alarm(30);
}
- ~AsyncUtilTest() {
+ virtual ~AsyncUtilTest() {
// Disable the alarm on test exit.
alarm(0);
}
@@ -99,31 +100,72 @@ TEST_F(AsyncUtilTest, TestSynchronizerMultiWait) {
}
}
-TEST_F(AsyncUtilTest, TestSynchronizerTimedWait) {
- thread waiter;
- {
- Synchronizer sync;
- auto cb = sync.AsStatusCallback();
- waiter = thread([cb] {
- SleepFor(MonoDelta::FromMilliseconds(5));
- cb.Run(Status::OK());
- });
- ASSERT_OK(sync.WaitFor(MonoDelta::FromMilliseconds(1000)));
- }
- waiter.join();
+// Flavors of wait that Synchronizer is capable of: WaitFor() or WaitUntil().
+enum class TimedWaitFlavor {
+ WaitFor,
+ WaitUntil,
+};
- {
- Synchronizer sync;
- auto cb = sync.AsStatusCallback();
- waiter = thread([cb] {
- SleepFor(MonoDelta::FromMilliseconds(1000));
- cb.Run(Status::OK());
- });
- ASSERT_TRUE(sync.WaitFor(MonoDelta::FromMilliseconds(5)).IsTimedOut());
+class AsyncUtilTimedWaitTest:
+ public AsyncUtilTest,
+ public ::testing::WithParamInterface<TimedWaitFlavor> {
+};
+
+TEST_P(AsyncUtilTimedWaitTest, SynchronizerTimedWaitSuccess) {
+ const auto kWaitInterval = MonoDelta::FromMilliseconds(1000);
+
+ Synchronizer sync;
+ auto cb = sync.AsStatusCallback();
+ auto waiter = thread([cb] {
+ SleepFor(MonoDelta::FromMilliseconds(5));
+ cb.Run(Status::OK());
+ });
+ SCOPED_CLEANUP({
+ waiter.join();
+ });
+ const auto mode = GetParam();
+ switch (mode) {
+ case TimedWaitFlavor::WaitFor:
+ ASSERT_OK(sync.WaitFor(kWaitInterval));
+ break;
+ case TimedWaitFlavor::WaitUntil:
+ ASSERT_OK(sync.WaitUntil(MonoTime::Now() + kWaitInterval));
+ break;
+ default:
+ FAIL() << "unsupported wait mode " << static_cast<int>(mode);
+ break;
}
+}
+
+TEST_P(AsyncUtilTimedWaitTest, SynchronizerTimedWaitTimeout) {
+ const auto kWaitInterval = MonoDelta::FromMilliseconds(5);
- // Waiting on the thread gives TSAN to check that no thread safety issues
- // occurred.
- waiter.join();
+ Synchronizer sync;
+ auto cb = sync.AsStatusCallback();
+ auto waiter = thread([cb] {
+ SleepFor(MonoDelta::FromMilliseconds(1000));
+ cb.Run(Status::OK());
+ });
+ SCOPED_CLEANUP({
+ waiter.join();
+ });
+ const auto mode = GetParam();
+ switch (mode) {
+ case TimedWaitFlavor::WaitFor:
+ ASSERT_TRUE(sync.WaitFor(kWaitInterval).IsTimedOut());
+ break;
+ case TimedWaitFlavor::WaitUntil:
+ ASSERT_TRUE(sync.WaitUntil(MonoTime::Now() + kWaitInterval).IsTimedOut());
+ break;
+ default:
+ FAIL() << "unsupported wait mode " << static_cast<int>(mode);
+ break;
+ }
}
+
+INSTANTIATE_TEST_CASE_P(WaitFlavors,
+ AsyncUtilTimedWaitTest,
+ ::testing::Values(TimedWaitFlavor::WaitFor,
+ TimedWaitFlavor::WaitUntil));
+
} // namespace kudu
diff --git a/src/kudu/util/async_util.h b/src/kudu/util/async_util.h
index 338c6c2..61621d6 100644
--- a/src/kudu/util/async_util.h
+++ b/src/kudu/util/async_util.h
@@ -44,7 +44,7 @@ namespace kudu {
class Synchronizer {
public:
Synchronizer()
- : data_(std::make_shared<Data>()) {
+ : data_(std::make_shared<Data>()) {
}
void StatusCB(const Status& status) {
@@ -71,6 +71,13 @@ class Synchronizer {
return data_->status;
}
+ Status WaitUntil(const MonoTime& deadline) const {
+ if (PREDICT_FALSE(!data_->latch.WaitUntil(deadline))) {
+ return Status::TimedOut("timed out while waiting for the callback to be called");
+ }
+ return data_->status;
+ }
+
void Reset() {
data_->latch.Reset(1);
}
[kudu] 01/03: KUDU-2711 pt 4. Java support for GetTableLocations
optimizations
Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 58f189dac6aa691bd7b8e5ebc1e89756385147a8
Author: Will Berkeley <wd...@gmail.com>
AuthorDate: Tue May 7 10:55:51 2019 -0700
KUDU-2711 pt 4. Java support for GetTableLocations optimizations
This adds support for the optimized GetTableLocations response format
added in 586e957f76a547340f2ab93a7eebc3f116ff825e.
There's no new tests, but as this changes the way GetTableLocations
works, it's tested by all existing tests that do any writes or scans, so
it's well-tested by existing tests.
Additionally, to test backwards compatibility, I ran the Java client
test suite while using a set of binaries compiled without support for
the GetTableLocations optimization.
Change-Id: I5af146fd1984ce683f056877129506cd2068e0e8
Reviewed-on: http://gerrit.cloudera.org:8080/13287
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Will Berkeley <wd...@gmail.com>
---
.../org/apache/kudu/client/AsyncKuduClient.java | 48 +++++++++++++++++++---
.../kudu/client/GetTableLocationsRequest.java | 1 +
.../java/org/apache/kudu/client/LocatedTablet.java | 2 +
.../java/org/apache/kudu/client/RemoteTablet.java | 14 ++++---
.../apache/kudu/client/TestAsyncKuduClient.java | 6 ++-
.../org/apache/kudu/client/TestRemoteTablet.java | 14 +++----
6 files changed, 63 insertions(+), 22 deletions(-)
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java b/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
index afc54d1..668f238 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
@@ -35,6 +35,7 @@ import java.net.UnknownHostException;
import java.security.cert.CertificateException;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.function.Consumer;
import java.util.HashSet;
import java.util.List;
import java.util.Random;
@@ -75,6 +76,7 @@ import org.apache.kudu.Schema;
import org.apache.kudu.master.Master;
import org.apache.kudu.master.Master.GetTableLocationsResponsePB;
import org.apache.kudu.master.Master.TableIdentifierPB;
+import org.apache.kudu.master.Master.TSInfoPB;
import org.apache.kudu.util.AsyncUtil;
import org.apache.kudu.util.NetUtil;
import org.apache.kudu.util.Pair;
@@ -2112,6 +2114,7 @@ public class AsyncKuduClient implements AutoCloseable {
partitionKey,
requestedBatchSize,
response.getTabletLocationsList(),
+ response.getTsInfosList(),
response.getTtlMillis());
} catch (KuduException e) {
return e;
@@ -2146,12 +2149,13 @@ public class AsyncKuduClient implements AutoCloseable {
}
/**
- * Makes discovered tablet locations visible in the clients caches.
+ * Makes discovered tablet locations visible in the client's caches.
* @param table the table which the locations belong to
* @param requestPartitionKey the partition key of the table locations request
* @param requestedBatchSize the number of tablet locations requested from the master in the
* original request
* @param locations the discovered locations
+ * @param tsInfosList a list of ts info that the replicas in 'locations' references by index.
* @param ttl the ttl of the locations
*/
@InterfaceAudience.LimitedPrivate("Test")
@@ -2159,8 +2163,8 @@ public class AsyncKuduClient implements AutoCloseable {
byte[] requestPartitionKey,
int requestedBatchSize,
List<Master.TabletLocationsPB> locations,
+ List<Master.TSInfoPB> tsInfosList,
long ttl) throws KuduException {
- // TODO(todd): handle "interned" response here
String tableId = table.getTableId();
String tableName = table.getName();
@@ -2179,20 +2183,48 @@ public class AsyncKuduClient implements AutoCloseable {
// Build the list of discovered remote tablet instances. If we have
// already discovered the tablet, its locations are refreshed.
+ int numTsInfos = tsInfosList.size();
List<RemoteTablet> tablets = new ArrayList<>(locations.size());
for (Master.TabletLocationsPB tabletPb : locations) {
- List<UnknownHostException> lookupExceptions = new ArrayList<>(tabletPb.getReplicasCount());
+ List<Exception> lookupExceptions = new ArrayList<>(tabletPb.getReplicasCount());
List<ServerInfo> servers = new ArrayList<>(tabletPb.getReplicasCount());
- for (Master.TabletLocationsPB.ReplicaPB replica : tabletPb.getReplicasList()) {
+
+ // Lambda that does the common handling of a ts info.
+ Consumer<Master.TSInfoPB> updateServersAndCollectExceptions = tsInfo -> {
try {
- ServerInfo serverInfo = resolveTS(replica.getTsInfo());
+ ServerInfo serverInfo = resolveTS(tsInfo);
if (serverInfo != null) {
servers.add(serverInfo);
}
} catch (UnknownHostException ex) {
lookupExceptions.add(ex);
}
+ };
+
+ // Handle "old-style" non-interned replicas.
+ for (Master.TabletLocationsPB.ReplicaPB replica : tabletPb.getReplicasList()) {
+ updateServersAndCollectExceptions.accept(replica.getTsInfo());
+ }
+
+ // Handle interned replicas. As a shim, we also need to create a list of "old-style" ReplicaPBs
+ // to be stored inside the RemoteTablet.
+ // TODO(wdberkeley): Change this so ReplicaPBs aren't used by the client at all anymore.
+ List<Master.TabletLocationsPB.ReplicaPB> replicas = new ArrayList<>();
+ for (Master.TabletLocationsPB.InternedReplicaPB replica : tabletPb.getInternedReplicasList()) {
+ int tsInfoIdx = replica.getTsInfoIdx();
+ if (tsInfoIdx >= numTsInfos) {
+ lookupExceptions.add(new NonRecoverableException(Status.Corruption(
+ String.format("invalid response from master: referenced tablet idx %d but only %d present",
+ tsInfoIdx, numTsInfos))));
+ continue;
+ }
+ TSInfoPB tsInfo = tsInfosList.get(tsInfoIdx);
+ updateServersAndCollectExceptions.accept(tsInfo);
+ Master.TabletLocationsPB.ReplicaPB.Builder builder = Master.TabletLocationsPB.ReplicaPB.newBuilder();
+ builder.setRole(replica.getRole());
+ builder.setTsInfo(tsInfo);
+ replicas.add(builder.build());
}
if (!lookupExceptions.isEmpty() &&
@@ -2202,7 +2234,11 @@ public class AsyncKuduClient implements AutoCloseable {
throw new NonRecoverableException(statusIOE);
}
- RemoteTablet rt = new RemoteTablet(tableId, tabletPb, servers);
+ RemoteTablet rt = new RemoteTablet(tableId,
+ tabletPb.getTabletId().toStringUtf8(),
+ ProtobufHelper.pbToPartition(tabletPb.getPartition()),
+ replicas.isEmpty() ? tabletPb.getReplicasList() : replicas,
+ servers);
LOG.debug("Learned about tablet {} for table '{}' with partition {}",
rt.getTabletId(), tableName, rt.getPartition());
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/GetTableLocationsRequest.java b/java/kudu-client/src/main/java/org/apache/kudu/client/GetTableLocationsRequest.java
index a7fe825..3be2770 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/GetTableLocationsRequest.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/GetTableLocationsRequest.java
@@ -91,6 +91,7 @@ class GetTableLocationsRequest extends KuduRpc<Master.GetTableLocationsResponseP
builder.setPartitionKeyEnd(UnsafeByteOperations.unsafeWrap(endKey));
}
builder.setMaxReturnedLocations(maxReturnedLocations);
+ builder.setInternTsInfosInResponse(true);
return builder.build();
}
}
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/LocatedTablet.java b/java/kudu-client/src/main/java/org/apache/kudu/client/LocatedTablet.java
index 655f800..ae31903 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/LocatedTablet.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/LocatedTablet.java
@@ -107,6 +107,8 @@ public class LocatedTablet {
@InterfaceAudience.Public
@InterfaceStability.Evolving
public static class Replica {
+ // TODO(wdberkeley): The ReplicaPB is deprecated server-side, so we ought to redo how this
+ // class stores its information.
private final ReplicaPB pb;
Replica(ReplicaPB pb) {
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java b/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
index fd0eb3a..2241f58 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
@@ -69,11 +69,13 @@ public class RemoteTablet implements Comparable<RemoteTablet> {
private String leaderUuid;
RemoteTablet(String tableId,
- Master.TabletLocationsPB tabletLocations,
+ String tabletId,
+ Partition partition,
+ List<Master.TabletLocationsPB.ReplicaPB> replicas,
List<ServerInfo> serverInfos) {
- this.tabletId = tabletLocations.getTabletId().toStringUtf8();
+ this.tabletId = tabletId;
this.tableId = tableId;
- this.partition = ProtobufHelper.pbToPartition(tabletLocations.getPartition());
+ this.partition = partition;
this.tabletServers = new HashMap<>(serverInfos.size());
for (ServerInfo serverInfo : serverInfos) {
@@ -81,18 +83,18 @@ public class RemoteTablet implements Comparable<RemoteTablet> {
}
ImmutableList.Builder<LocatedTablet.Replica> replicasBuilder = new ImmutableList.Builder<>();
- for (Master.TabletLocationsPB.ReplicaPB replica : tabletLocations.getReplicasList()) {
+ for (Master.TabletLocationsPB.ReplicaPB replica : replicas) {
String uuid = replica.getTsInfo().getPermanentUuid().toStringUtf8();
replicasBuilder.add(new LocatedTablet.Replica(replica));
if (replica.getRole().equals(Metadata.RaftPeerPB.Role.LEADER)) {
- leaderUuid = uuid;
+ this.leaderUuid = uuid;
}
}
if (leaderUuid == null) {
LOG.warn("No leader provided for tablet {}", getTabletId());
}
- replicas.set(replicasBuilder.build());
+ this.replicas.set(replicasBuilder.build());
}
@Override
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java
index d79d114..bef643d 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java
@@ -173,7 +173,8 @@ public class TestAsyncKuduClient {
try {
KuduTable badTable = new KuduTable(asyncClient, "Invalid table name",
"Invalid table ID", null, null, 3);
- asyncClient.discoverTablets(badTable, null, requestBatchSize, tabletLocations, 1000);
+ asyncClient.discoverTablets(badTable, null, requestBatchSize,
+ tabletLocations, new ArrayList<>(), 1000);
fail("This should have failed quickly");
} catch (NonRecoverableException ex) {
assertTrue(ex.getMessage().contains(badHostname));
@@ -205,7 +206,8 @@ public class TestAsyncKuduClient {
"master", leader.getRpcHost(), leader.getRpcPort(), Metadata.RaftPeerPB.Role.FOLLOWER));
tabletLocations.add(tabletPb.build());
try {
- asyncClient.discoverTablets(table, new byte[0], requestBatchSize, tabletLocations, 1000);
+ asyncClient.discoverTablets(table, new byte[0], requestBatchSize,
+ tabletLocations, new ArrayList<>(), 1000);
fail("discoverTablets should throw an exception if there's no leader");
} catch (NoLeaderFoundException ex) {
// Expected.
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java
index 43ca37a..c4372c1 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestRemoteTablet.java
@@ -223,10 +223,8 @@ public class TestRemoteTablet {
static RemoteTablet getTablet(int leaderIndex,
int localReplicaIndex,
int sameLocationReplicaIndex) {
- Master.TabletLocationsPB.Builder tabletPb = Master.TabletLocationsPB.newBuilder();
-
- tabletPb.setPartition(ProtobufUtils.getFakePartitionPB());
- tabletPb.setTabletId(ByteString.copyFromUtf8("fake tablet"));
+ Partition partition = ProtobufHelper.pbToPartition(ProtobufUtils.getFakePartitionPB().build());
+ List<Master.TabletLocationsPB.ReplicaPB> replicas = new ArrayList<>();
List<ServerInfo> servers = new ArrayList<>();
for (int i = 0; i < 3; i++) {
InetAddress addr;
@@ -246,11 +244,11 @@ public class TestRemoteTablet {
new HostAndPort("host", 1000 + i),
addr,
location));
- tabletPb.addReplicas(ProtobufUtils.getFakeTabletReplicaPB(
- uuid, "host", i,
- leaderIndex == i ? Metadata.RaftPeerPB.Role.LEADER : Metadata.RaftPeerPB.Role.FOLLOWER));
+ Metadata.RaftPeerPB.Role role = leaderIndex == i ? Metadata.RaftPeerPB.Role.LEADER :
+ Metadata.RaftPeerPB.Role.FOLLOWER;
+ replicas.add(ProtobufUtils.getFakeTabletReplicaPB(uuid, "host", i, role).build());
}
- return new RemoteTablet("fake table", tabletPb.build(), servers);
+ return new RemoteTablet("fake table", "fake tablet", partition, replicas, servers);
}
}