Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
601bb27
Replace 101tec ZkClient with Helix ZkClient
Nov 9, 2021
065450e
Use topic level throughput information when partition level informati…
vmaheshw Nov 29, 2021
b838a4b
Fix compilation errors (#874)
vmaheshw Dec 1, 2021
9906eca
Removing unused older zk client version number variable and redundant…
Dec 2, 2021
5a09717
Add exception unit tests for ZkClient create method
Dec 4, 2021
49554b4
Update log message and refactor unit tests
Dec 6, 2021
7380202
Loadbased Partition assigner not using topic level metrics to recogni…
vmaheshw Dec 6, 2021
7f9c97d
Updating Zookeeper version to 3.6.2.3 which has pagination support (3…
Dec 14, 2021
039c28c
Flushless producer supporting both comparable and non comparable offs…
shrinandthakkar Jan 5, 2022
87c7bec
Fix liveInstance provider call only in leader node (#879)
vmaheshw Jan 25, 2022
2cd2b8b
Fixed issue with missing exception message during task initialization…
jzakaryan Feb 1, 2022
4d867ef
Kafka upgrade (#881)
srinagaraj Feb 2, 2022
37544cf
skipping onPartitionsRevoked during consumer.close() call (#886)
srinagaraj Mar 9, 2022
d64e248
Scale 2.12 upgrade (#895)
srinagaraj Mar 9, 2022
dbaffa6
Upgrade avro and move jackson from codehaus to fasterxml (#894)
vmaheshw Mar 9, 2022
26f32ca
Fix topic deletion when multiple duplicate streams expire at the same…
Mar 14, 2022
4310926
Use 2.4.1.57 version open source kafka (#901)
shrinandthakkar Mar 31, 2022
a67108b
tests for min/max partitions per task metrics and minor code quality …
ryannedolan Apr 12, 2022
b821911
fix rebalancing-tasks bug and added tests (#900)
shrinandthakkar Apr 12, 2022
4f806ac
Refactor Stopping Tasks On Assignment Change of Tasks (#868)
shrinandthakkar Apr 12, 2022
f6c7e76
Change python dependency in commit-msg git hook (#904)
Apr 18, 2022
a34bbdd
Remove Scala Dependencies (#905)
shrinandthakkar Apr 21, 2022
55027bc
Introduce broadcast API to TransportProvider (#903)
May 23, 2022
981a32f
Dedupe tasks on LeaderDoAssignment (#906)
jogrogan Jun 6, 2022
5b30f54
Fix Stopping Logic and Maintain Stopping Latch Counter (#877)
shrinandthakkar Jun 8, 2022
51533bb
Fixing assignment change test to include task thread dead parameter a…
shrinandthakkar Jun 9, 2022
5657225
Replace 101tec ZkClient with Helix ZkClient
Nov 9, 2021
cdc0c6c
Removing unused older zk client version number variable and redundant…
Dec 2, 2021
f4d6dcf
Add exception unit tests for ZkClient create method
Dec 4, 2021
608ec07
Update log message and refactor unit tests
Dec 6, 2021
50cadd4
Updating Zookeeper version to 3.6.2.3 which has pagination support (3…
Dec 14, 2021
f06e55b
Revert zookeeper version and update major version of brooklin
Jun 24, 2022
6b5f2f3
Merge branch 'helix_zkclient' of github.com:surajkn/brooklin into hel…
Jun 24, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ project(':datastream-server-api') {
project(':datastream-utils') {
dependencies {
compile project(':datastream-common')
compile "com.101tec:zkclient:$zkclientVersion"
compile "org.apache.helix:zookeeper-api:$helixZkclientVersion"
compile "com.google.guava:guava:$guavaVersion"
testCompile project(":datastream-kafka_$scalaSuffix")
testCompile project(":datastream-testcommon_$scalaSuffix")
Expand Down Expand Up @@ -319,7 +319,6 @@ project(':datastream-client') {
project(':datastream-server') {

dependencies {
compile "com.101tec:zkclient:$zkclientVersion"
compile "org.codehaus.jackson:jackson-core-asl:$jacksonVersion"
compile "org.codehaus.jackson:jackson-mapper-asl:$jacksonVersion"

Expand Down Expand Up @@ -355,7 +354,6 @@ project(':datastream-server-restli') {
compile "com.linkedin.pegasus:restli-netty-standalone:$pegasusVersion"
compile "com.linkedin.pegasus:r2-jetty:$pegasusVersion"
compile "com.linkedin.parseq:parseq:$parseqVersion"
compile "com.101tec:zkclient:$zkclientVersion"
compile "org.codehaus.jackson:jackson-core-asl:$jacksonVersion"
compile "org.codehaus.jackson:jackson-mapper-asl:$jacksonVersion"
compile "org.apache.commons:commons-lang3:$commonslang3Version"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
import java.util.regex.Pattern;
import java.util.stream.Collectors;

import org.I0Itec.zkclient.exception.ZkInterruptedException;
import org.apache.commons.lang3.StringUtils;
import org.apache.helix.zookeeper.zkclient.exception.ZkInterruptedException;
import org.apache.kafka.clients.consumer.Consumer;
import org.apache.kafka.clients.consumer.ConsumerConfig;
import org.apache.kafka.clients.consumer.ConsumerRecord;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import java.util.concurrent.ConcurrentHashMap;
import java.util.stream.Collectors;

import org.I0Itec.zkclient.exception.ZkNoNodeException;
import org.apache.helix.zookeeper.zkclient.exception.ZkNoNodeException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@
import java.util.function.Function;
import java.util.stream.Collectors;

import org.I0Itec.zkclient.IZkChildListener;
import org.I0Itec.zkclient.IZkDataListener;
import org.I0Itec.zkclient.IZkStateListener;
import org.I0Itec.zkclient.exception.ZkException;
import org.I0Itec.zkclient.exception.ZkNoNodeException;
import org.apache.commons.lang3.StringUtils;
import org.apache.helix.zookeeper.zkclient.IZkChildListener;
import org.apache.helix.zookeeper.zkclient.IZkDataListener;
import org.apache.helix.zookeeper.zkclient.IZkStateListener;
import org.apache.helix.zookeeper.zkclient.exception.ZkException;
import org.apache.helix.zookeeper.zkclient.exception.ZkNoNodeException;
import org.apache.zookeeper.CreateMode;
import org.apache.zookeeper.Watcher;
import org.slf4j.Logger;
Expand Down Expand Up @@ -1835,7 +1835,7 @@ public void handleStateChanged(Watcher.Event.KeeperState state) {
}

@Override
public void handleNewSession() {
public void handleNewSession(final String sessionId) {
synchronized (_zkSessionLock) {
LOG.info("ZkStateChangeListener::A new session has been established.");
if (_reinitOnNewSession) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,35 +6,29 @@
package com.linkedin.datastream.common.zk;

import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.List;
import java.util.Random;
import java.util.Stack;

import org.I0Itec.zkclient.ZkConnection;
import org.I0Itec.zkclient.exception.ZkInterruptedException;
import org.I0Itec.zkclient.exception.ZkMarshallingError;
import org.I0Itec.zkclient.exception.ZkNoNodeException;
import org.I0Itec.zkclient.exception.ZkNodeExistsException;
import org.I0Itec.zkclient.serialize.ZkSerializer;
import org.apache.zookeeper.CreateMode;
import org.apache.zookeeper.data.Stat;
import org.apache.helix.zookeeper.zkclient.exception.ZkMarshallingError;
import org.apache.helix.zookeeper.zkclient.exception.ZkNodeExistsException;
import org.apache.helix.zookeeper.zkclient.serialize.ZkSerializer;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.google.common.annotations.VisibleForTesting;


/**
* ZKClient is a wrapper of {@link org.I0Itec.zkclient.ZkClient}. It provides the following
* ZKClient is a wrapper of {@link org.apache.helix.zookeeper.impl.client.ZkClient}. It provides the following
* basic features:
* <ol>
* <li>tolerate network reconnects so the caller doesn't have to handle the retries</li>
* <li>provide a String serializer since we only need to store JSON strings in ZooKeeper</li>
* <li>additional features like ensurePath to recursively create paths</li>
* </ol>
*/
public class ZkClient extends org.I0Itec.zkclient.ZkClient {
public class ZkClient extends org.apache.helix.zookeeper.impl.client.ZkClient {
public static final String ZK_PATH_SEPARATOR = "/";
public static final int DEFAULT_CONNECTION_TIMEOUT = 60 * 1000;
public static final int DEFAULT_SESSION_TIMEOUT = 30 * 1000;
Expand Down Expand Up @@ -86,71 +80,20 @@ public ZkClient(String zkServers, int sessionTimeoutMs, int connectionTimeoutMs,
_zkSessionTimeoutMs = sessionTimeoutMs;
}

@Override
Comment thread
surajkn marked this conversation as resolved.
public void close() throws ZkInterruptedException {
if (LOG.isTraceEnabled()) {
StackTraceElement[] calls = Thread.currentThread().getStackTrace();
LOG.trace("closing zkclient. callStack: {}", Arrays.asList(calls));
}
getEventLock().lock();
try {
if (_connection == null) {
return;
}
LOG.info("closing zkclient: {}", ((ZkConnection) _connection).getZookeeper());
super.close();
} catch (ZkInterruptedException e) {
/*
* Workaround for HELIX-264: calling ZkClient#disconnect() in its own eventThread context will
* throw ZkInterruptedException and skip ZkConnection#disconnect()
*/
try {
/*
* ZkInterruptedException#construct() honors InterruptedException by calling
* Thread.currentThread().interrupt(); clear it first, so we can safely disconnect the
* zk-connection
*/
Thread.interrupted();
_connection.close();
/*
* restore interrupted status of current thread
*/
Thread.currentThread().interrupt();
} catch (InterruptedException e1) {
throw new ZkInterruptedException(e1);
}
} finally {
getEventLock().unlock();
LOG.info("closed zkclient");
}
}

/**
* Check if a zk path exists. Changes the access modified to public, its defined as protected in parent class.
*/
@Override
public boolean exists(final String path, final boolean watch) {
long startT = System.nanoTime();

try {
return retryUntilConnected(() -> _connection.exists(path, watch));
} finally {
long endT = System.nanoTime();
if (LOG.isTraceEnabled()) {
LOG.trace("exists, path: {}, time: {} ns", path, (endT - startT));
}
}
return super.exists(path, watch);
}

/**
* Get all children of zk path. Changes the access modified to public, its defined as protected in parent class.
*/
@Override
public List<String> getChildren(final String path, final boolean watch) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can probably get rid of this method itself since you are only calling super.()

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method is declared as protected in parent class and here I am just expanding the access specifier.
For now I thought to keep this around and later I can investigate where all this is used (across all MPs) and then see if it can be changed to use just the "public" getChildren method in parent class. Same for a few other methods that I have done this way.

long startT = System.nanoTime();

try {
return retryUntilConnected(() -> _connection.getChildren(path, watch));
} finally {
long endT = System.nanoTime();
if (LOG.isTraceEnabled()) {
LOG.trace("getChildren, path: {}, time: {} ns", path, (endT - startT));
}
}
return super.getChildren(path, watch);
}

Comment thread
surajkn marked this conversation as resolved.
/**
Expand Down Expand Up @@ -212,80 +155,6 @@ public String ensureReadData(final String path) {
return ensureReadData(path, _zkSessionTimeoutMs);
}

@Override
@SuppressWarnings("unchecked")
protected <T extends Object> T readData(final String path, final Stat stat, final boolean watch) {
long startT = System.nanoTime();
try {
byte[] data = retryUntilConnected(() -> _connection.readData(path, stat, watch));
return (T) deserialize(data);
} finally {
long endT = System.nanoTime();
if (LOG.isTraceEnabled()) {
LOG.trace("readData, path: {}, time: {} ns", path, (endT - startT));
}
}
}

@Override
public void writeData(final String path, Object data, final int expectedVersion) {
long startT = System.nanoTime();
try {
final byte[] bytes = serialize(data);

retryUntilConnected(() -> {
_connection.writeData(path, bytes, expectedVersion);
return null;
});
} finally {
long endT = System.nanoTime();
if (LOG.isTraceEnabled()) {
LOG.trace("writeData, path: {}, time: {} ns", path, (endT - startT));
}
}
}

@Override
public String create(final String path, Object data, final CreateMode mode) throws RuntimeException {
if (path == null) {
throw new IllegalArgumentException("path must not be null.");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new library throws NullPointerException instead of IllegalArgumentException. Can you plz check the contract of new methods ? Mostly the unit-tests are not covering it, otherwise the tests should have failed. May be a good idea to add those tests

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, this is something to bring up with Helix team that their contact says IllegalArgumentException but internally the method throws NullPointerException.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added unit test for NullPointerException. Will bring up the discrepancy with Helix team separately

}

long startT = System.nanoTime();
try {
final byte[] bytes = data == null ? null : serialize(data);

return retryUntilConnected(() -> _connection.create(path, bytes, mode));
} finally {
long endT = System.nanoTime();
if (LOG.isTraceEnabled()) {
LOG.trace("create, path: {}, time: {} ns", path, (endT - startT));
}
}
}

@Override
public boolean delete(final String path) {
long startT = System.nanoTime();
try {
try {
retryUntilConnected(() -> {
_connection.delete(path);
return null;
});

return true;
} catch (ZkNoNodeException e) {
return false;
}
} finally {
long endT = System.nanoTime();
if (LOG.isTraceEnabled()) {
LOG.trace("delete, path: {}, time: {} ns", path, (endT - startT));
}
}
}

/**
* Ensure that all the paths in the given full path String are created
* @param path the zk path
Expand Down Expand Up @@ -347,9 +216,12 @@ public <T extends Object> T deserialize(byte[] data) {
return (T) _zkSerializer.deserialize(data);
}

/**
* Get Zk sessions session ID
Comment thread
surajkn marked this conversation as resolved.
Outdated
*/
@VisibleForTesting
public long getSessionId() {
Comment thread
surajkn marked this conversation as resolved.
Outdated
return ((ZkConnection) _connection).getZookeeper().getSessionId();
return super.getSessionId();
}

private static class ZKStringSerializer implements ZkSerializer {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
import java.io.IOException;
import java.util.List;

import org.I0Itec.zkclient.IZkChildListener;
import org.I0Itec.zkclient.IZkDataListener;
import org.apache.helix.zookeeper.zkclient.IZkChildListener;
import org.apache.helix.zookeeper.zkclient.IZkDataListener;
import org.apache.zookeeper.CreateMode;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down
2 changes: 1 addition & 1 deletion gradle/dependency-versions.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,5 @@ ext {
testngVersion = "7.1.0"
zkclientVersion = "0.11"
Comment thread
surajkn marked this conversation as resolved.
Outdated
zookeeperVersion = "3.4.13"
helixZkclientVersion = "1.0.1"
helixZkclientVersion = "1.0.2"
}