[
https://issues.apache.org/jira/browse/KNOX-3146?focusedWorklogId=972020&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-972020
]
ASF GitHub Bot logged work on KNOX-3146:
----------------------------------------
Author: ASF GitHub Bot
Created on: 05/Jun/25 20:11
Start Date: 05/Jun/25 20:11
Worklog Time Spent: 10m
Work Description: lmccay commented on code in PR #1049:
URL: https://github.com/apache/knox/pull/1049#discussion_r2130110688
##########
gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/CommonHaDispatch.java:
##########
@@ -38,14 +40,17 @@
import java.util.List;
import java.util.Map;
import java.util.Optional;
+import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
import java.util.stream.Collectors;
-public interface LBHaDispatch {
+public interface CommonHaDispatch {
Review Comment:
Not sure this was needed to be renamed. Failover is typically a LB feature
as well. Nothing you need to change but it probably would have required fewer
other changes to use the new name everywhere.
##########
gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/CommonHaDispatch.java:
##########
@@ -240,4 +270,73 @@ default void shiftActiveURL(boolean userAgentDisabled,
Optional<URI> backendURI)
}
}
}
+
Review Comment:
It is super confusing to have so much code in an interface.
Maybe only so for old eyes like mine though.
Based on the default modifier being in this class previously, I assume that
you are not introducing it here for the first time. I'll think about it a bit
more further into the review where multiple inheritance starts to rear its
head. :)
##########
gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/SSEHaDispatch.java:
##########
@@ -136,17 +149,100 @@ public void setActiveURL(String url) {
activeURL.set(url);
}
+ @Override
+ public int getMaxFailoverAttempts() {
+ return maxFailoverAttempts;
+ }
+
+ @Override
+ public void setMaxFailoverAttempts(int maxFailoverAttempts) {
+ this.maxFailoverAttempts = maxFailoverAttempts;
+ }
+
+ @Override
+ public int getFailoverSleep() {
+ return failoverSleep;
+ }
+
+ @Override
+ public void setFailoverSleep(int failoverSleep) {
+ this.failoverSleep = failoverSleep;
+ }
+
+ @Override
+ public void setFailoverNonIdempotentRequestEnabled(boolean enabled) {
+ this.failoverNonIdempotentRequestEnabled = enabled;
+ }
+
+ @Override
+ public boolean isFailoverNonIdempotentRequestEnabled() {
+ return failoverNonIdempotentRequestEnabled;
+ }
+
+ @Override
+ public void setNoFallbackEnabled(boolean enabled) {
+ this.noFallbackEnabled = enabled;
+ }
+
+ @Override
+ public boolean isNoFallbackEnabled() {
+ return noFallbackEnabled;
+ }
+
Review Comment:
Is this only because SSEHaDispatch doesn't extend the common one?
##########
gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/SSEHaDispatch.java:
##########
@@ -136,17 +149,100 @@ public void setActiveURL(String url) {
activeURL.set(url);
}
+ @Override
+ public int getMaxFailoverAttempts() {
+ return maxFailoverAttempts;
+ }
+
+ @Override
+ public void setMaxFailoverAttempts(int maxFailoverAttempts) {
+ this.maxFailoverAttempts = maxFailoverAttempts;
+ }
+
+ @Override
+ public int getFailoverSleep() {
+ return failoverSleep;
+ }
+
+ @Override
+ public void setFailoverSleep(int failoverSleep) {
+ this.failoverSleep = failoverSleep;
+ }
+
+ @Override
+ public void setFailoverNonIdempotentRequestEnabled(boolean enabled) {
+ this.failoverNonIdempotentRequestEnabled = enabled;
+ }
+
+ @Override
+ public boolean isFailoverNonIdempotentRequestEnabled() {
+ return failoverNonIdempotentRequestEnabled;
+ }
+
+ @Override
+ public void setNoFallbackEnabled(boolean enabled) {
+ this.noFallbackEnabled = enabled;
+ }
+
+ @Override
+ public boolean isNoFallbackEnabled() {
+ return noFallbackEnabled;
+ }
+
Review Comment:
All of the above methods have to be duplicated across all implementers of
CommonHaDispatch? Why is that? If they were added to a common extension instead
wouldn't that be better?
Issue Time Tracking
-------------------
Worklog Id: (was: 972020)
Time Spent: 20m (was: 10m)
> Failover ability for SSEHaDispatch
> ----------------------------------
>
> Key: KNOX-3146
> URL: https://issues.apache.org/jira/browse/KNOX-3146
> Project: Apache Knox
> Issue Type: Improvement
> Components: Server
> Affects Versions: 2.2.0
> Reporter: Tamás Hanicz
> Assignee: Tamás Hanicz
> Priority: Major
> Time Spent: 20m
> Remaining Estimate: 0h
>
> The failover ability is missing for SSEHaDispatch. It would be beneficial to
> add it.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
