Skip to content

Enable adaptive refresh by default #3249 #3316

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,19 @@
* Options to control the Cluster topology refreshing of {@link RedisClusterClient}.
*
* @author Mark Paluch
* @author Tihomir Mateev
* @since 4.2
*/
public class ClusterTopologyRefreshOptions {

public static final Set<RefreshTrigger> DEFAULT_ADAPTIVE_REFRESH_TRIGGERS = Collections.emptySet();
/**
* Since Lettuce 7.0 all adaptive triggers are enabled by default
*/
public static final Set<RefreshTrigger> DEFAULT_ADAPTIVE_REFRESH_TRIGGERS = EnumSet.allOf(RefreshTrigger.class);

public static final long DEFAULT_ADAPTIVE_REFRESH_TIMEOUT = 30;

@Deprecated
public static final TimeUnit DEFAULT_ADAPTIVE_REFRESH_TIMEOUT_UNIT = TimeUnit.SECONDS;

public static final Duration DEFAULT_ADAPTIVE_REFRESH_TIMEOUT_DURATION = Duration
Expand All @@ -54,6 +59,7 @@ public class ClusterTopologyRefreshOptions {

public static final long DEFAULT_REFRESH_PERIOD = 60;

@Deprecated
public static final TimeUnit DEFAULT_REFRESH_PERIOD_UNIT = TimeUnit.SECONDS;

public static final Duration DEFAULT_REFRESH_PERIOD_DURATION = Duration.ofSeconds(DEFAULT_REFRESH_PERIOD);
Expand Down Expand Up @@ -159,12 +165,16 @@ private Builder() {
* Enables adaptive topology refreshing using one or more {@link RefreshTrigger triggers}. Adaptive refresh triggers
* initiate topology view updates based on events happened during Redis Cluster operations. Adaptive triggers lead to an
* immediate topology refresh. Adaptive triggered refreshes are rate-limited using a timeout since events can happen on
* a large scale. Adaptive refresh triggers are disabled by default. See also
* a large scale. Adaptive refresh triggers are all enabled by default. See also
* {@link #adaptiveRefreshTriggersTimeout(long, TimeUnit)} and {@link RefreshTrigger}.
*
* @param refreshTrigger one or more {@link RefreshTrigger} to enabled
* @return {@code this}
* @deprecated Starting from 7.0, this method has no effect as all adaptive triggers are enabled by default.
* @see #disableAllAdaptiveRefreshTriggers()
* @see #disableAdaptiveRefreshTrigger(RefreshTrigger...)
*/
@Deprecated
public Builder enableAdaptiveRefreshTrigger(RefreshTrigger... refreshTrigger) {

LettuceAssert.notNull(refreshTrigger, "RefreshTriggers must not be null");
Expand All @@ -179,16 +189,56 @@ public Builder enableAdaptiveRefreshTrigger(RefreshTrigger... refreshTrigger) {
* Enables adaptive topology refreshing using all {@link RefreshTrigger triggers}. Adaptive refresh triggers initiate
* topology view updates based on events happened during Redis Cluster operations. Adaptive triggers lead to an
* immediate topology refresh. Adaptive triggered refreshes are rate-limited using a timeout since events can happen on
* a large scale. Adaptive refresh triggers are disabled by default. See also
* a large scale. Adaptive refresh triggers are all enabled by default. See also
* {@link #adaptiveRefreshTriggersTimeout(long, TimeUnit)} and {@link RefreshTrigger}.
*
* @return {@code this}
* @deprecated Starting from 7.0, this method has no effect as all adaptive triggers are enabled by default.
* @see #disableAllAdaptiveRefreshTriggers()
* @see #disableAdaptiveRefreshTrigger(RefreshTrigger...)
*/
@Deprecated
public Builder enableAllAdaptiveRefreshTriggers() {
adaptiveRefreshTriggers.addAll(EnumSet.allOf(RefreshTrigger.class));
return this;
}

/**
* Disables adaptive topology refreshing using one or more {@link RefreshTrigger triggers}. Adaptive refresh triggers
* initiate topology view updates based on events happened during Redis Cluster operations. Adaptive triggers lead to an
* immediate topology refresh. Adaptive triggered refreshes are rate-limited using a timeout since events can happen on
* a large scale. Adaptive refresh triggers are all enabled by default. See also
* {@link #adaptiveRefreshTriggersTimeout(long, TimeUnit)} and {@link RefreshTrigger}.
*
* @param refreshTrigger one or more {@link RefreshTrigger} to enabled
* @return {@code this}
* @since 7.0
*/
public Builder disableAdaptiveRefreshTrigger(RefreshTrigger... refreshTrigger) {

LettuceAssert.notNull(refreshTrigger, "RefreshTriggers must not be null");
LettuceAssert.noNullElements(refreshTrigger, "RefreshTriggers must not contain null elements");
LettuceAssert.notEmpty(refreshTrigger, "RefreshTriggers must contain at least one element");

Arrays.asList(refreshTrigger).forEach(adaptiveRefreshTriggers::remove);
return this;
}

/**
* Disables adaptive topology refreshing using all {@link RefreshTrigger triggers}. Adaptive refresh triggers initiate
* topology view updates based on events happened during Redis Cluster operations. Adaptive triggers lead to an
* immediate topology refresh. Adaptive triggered refreshes are rate-limited using a timeout since events can happen on
* a large scale. Adaptive refresh triggers are all enabled by default. See also
* {@link #adaptiveRefreshTriggersTimeout(long, TimeUnit)} and {@link RefreshTrigger}.
*
* @return {@code this}
* @since 7.0
*/
public Builder disableAllAdaptiveRefreshTriggers() {
adaptiveRefreshTriggers.clear();
return this;
}

/**
* Set the timeout for adaptive topology updates. This timeout is to rate-limit topology updates initiated by refresh
* triggers to one topology refresh per timeout. Defaults to {@literal 30 SECONDS}. See {@link #DEFAULT_REFRESH_PERIOD}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ void testBuilder() {
ClusterTopologyRefreshOptions options = ClusterTopologyRefreshOptions.builder()//
.enablePeriodicRefresh(true).refreshPeriod(10, TimeUnit.MINUTES)//
.dynamicRefreshSources(false) //
.disableAllAdaptiveRefreshTriggers()//
.enableAdaptiveRefreshTrigger(RefreshTrigger.MOVED_REDIRECT)//
.adaptiveRefreshTriggersTimeout(15, TimeUnit.MILLISECONDS)//
.closeStaleConnections(false)//
Expand All @@ -46,6 +47,7 @@ void testCopy() {
ClusterTopologyRefreshOptions master = ClusterTopologyRefreshOptions.builder()//
.enablePeriodicRefresh(true).refreshPeriod(10, TimeUnit.MINUTES)//
.dynamicRefreshSources(false) //
.disableAllAdaptiveRefreshTriggers()//
.enableAdaptiveRefreshTrigger(RefreshTrigger.MOVED_REDIRECT)//
.adaptiveRefreshTriggersTimeout(15, TimeUnit.MILLISECONDS)//
.closeStaleConnections(false)//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ void shouldTriggerRefreshOnAskRedirection() {
}

@Test
void shouldNotTriggerAdaptiveRefreshUsingDefaults() {
void shouldTriggerAdaptiveRefreshUsingDefaults() {

ClusterTopologyRefreshOptions clusterTopologyRefreshOptions = ClusterTopologyRefreshOptions.create();

Expand All @@ -141,6 +141,21 @@ void shouldNotTriggerAdaptiveRefreshUsingDefaults() {

when(clusterClient.getClusterClientOptions()).thenReturn(clusterClientOptions);

sut.onAskRedirection();
verify(eventExecutors).submit(any(Runnable.class));
}

@Test
void shouldNotTriggerAdaptiveRefreshWhenDisabled() {

ClusterTopologyRefreshOptions clusterTopologyRefreshOptions = ClusterTopologyRefreshOptions.builder()
.disableAllAdaptiveRefreshTriggers().build();

ClusterClientOptions clusterClientOptions = ClusterClientOptions.builder()
.topologyRefreshOptions(clusterTopologyRefreshOptions).build();

when(clusterClient.getClusterClientOptions()).thenReturn(clusterClientOptions);

sut.onAskRedirection();
verify(eventExecutors, never()).submit(any(Runnable.class));
}
Expand Down
Loading