apurtell commented on code in PR #2489:
URL: https://github.com/apache/phoenix/pull/2489#discussion_r3463752469
##########
phoenix-core-server/src/main/java/org/apache/phoenix/replication/reader/ReplicationLogReplayService.java:
##########
@@ -77,14 +81,35 @@ public class ReplicationLogReplayService {
*/
public static final int
DEFAULT_REPLICATION_REPLAY_SERVICE_EXECUTOR_SHUTDOWN_TIMEOUT_SECONDS = 30;
+ private static final long CONSISTENCY_POINT_CACHE_TTL_SECONDS = 30;
+
private static volatile ReplicationLogReplayService instance;
private final Configuration conf;
private ScheduledExecutorService scheduler;
private volatile boolean isRunning = false;
+ private final Supplier<Long> cachedConsistencyPoint;
private ReplicationLogReplayService(final Configuration conf) {
this.conf = conf;
+ this.cachedConsistencyPoint = Suppliers.memoizeWithExpiration(() -> {
+ try {
+ return getConsistencyPoint();
+ } catch (IOException | SQLException e) {
+ throw new RuntimeException("Failed to fetch consistency point", e);
+ }
+ }, CONSISTENCY_POINT_CACHE_TTL_SECONDS, TimeUnit.SECONDS);
+ }
+
+ private ReplicationLogReplayService(long fixedConsistencyPoint) {
+ this.conf = null;
Review Comment:
The robot says
Any code path on the test singleton that touches conf will NPE:
start() reads conf.getBoolean(...) (line 155)
stop() reads conf.getBoolean(...) (line 190)
getReplicationGroups() calls HAGroupStoreManager.getInstance(conf) (line 295)
getReplicationLogReplay(...) calls ReplicationLogReplay.get(conf, ...) (line
299)
Today this is safe only because the test ITs and UTs go straight through
cachedConsistencyPoint.get() and never call the lifecycle methods.
##########
phoenix-core-server/src/main/java/org/apache/phoenix/replication/reader/ReplicationLogReplayService.java:
##########
@@ -77,14 +81,35 @@ public class ReplicationLogReplayService {
*/
public static final int
DEFAULT_REPLICATION_REPLAY_SERVICE_EXECUTOR_SHUTDOWN_TIMEOUT_SECONDS = 30;
+ private static final long CONSISTENCY_POINT_CACHE_TTL_SECONDS = 30;
+
private static volatile ReplicationLogReplayService instance;
private final Configuration conf;
private ScheduledExecutorService scheduler;
private volatile boolean isRunning = false;
+ private final Supplier<Long> cachedConsistencyPoint;
private ReplicationLogReplayService(final Configuration conf) {
this.conf = conf;
+ this.cachedConsistencyPoint = Suppliers.memoizeWithExpiration(() -> {
+ try {
+ return getConsistencyPoint();
+ } catch (IOException | SQLException e) {
+ throw new RuntimeException("Failed to fetch consistency point", e);
+ }
+ }, CONSISTENCY_POINT_CACHE_TTL_SECONDS, TimeUnit.SECONDS);
+ }
+
+ private ReplicationLogReplayService(long fixedConsistencyPoint) {
+ this.conf = null;
+ this.cachedConsistencyPoint = () -> fixedConsistencyPoint;
+ }
+
+ private ReplicationLogReplayService(Supplier<Long> supplier) {
+ this.conf = null;
Review Comment:
See above
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
