Copilot commented on code in PR #2607:
URL: https://github.com/apache/phoenix/pull/2607#discussion_r3918163667
##########
phoenix-core-server/src/main/java/org/apache/phoenix/replication/reader/ReplicationLogDiscoveryReplay.java:
##########
@@ -228,12 +228,85 @@ public void init() throws IOException {
}
@Override
- protected void processFile(Path path) throws IOException {
- LOG.info("Starting to process file {}", path);
+ protected void processFile(Path path, boolean firstClaim) throws IOException
{
+ LOG.info("Starting to process file {} (firstClaim={})", path, firstClaim);
+ ReplicationLogTracker tracker = getReplicationLogFileTracker();
+ final long fileTimestamp;
+ try {
+ fileTimestamp = tracker.getFileTimestamp(path);
+ } catch (NumberFormatException e) {
+ // A malformed file name cannot be anchored to a round. getFileTimestamp
is validated on the
+ // new-files path (getNewFilesForRound skips names that fail to parse)
but not on the reclaim
+ // path, so convert the unchecked parse failure into the IOException
that every other per-file
+ // failure uses. That keeps a single bad name isolated to
processOneRandomFile's catch (marked
+ // failed and retry-counted) instead of escaping as a RuntimeException
that aborts the whole
+ // in-progress sweep for the cycle.
+ throw new IOException("Cannot extract timestamp from replication log
file name: " + path, e);
Review Comment:
Parsing the filename now gates replay itself: an in-progress `.plog` with a
nonnumeric first segment was previously passed to `ReplicationLogProcessor`,
but this new exception causes `processOneRandomFile` to mark it failed without
reading it. That contradicts the PR's “no behavioral change” scope. Treat an
unparseable timestamp as “lag unavailable,” warn, and still replay the file;
update the new malformed-name test to assert replay occurs with no lag samples.
##########
phoenix-core-server/src/main/java/org/apache/phoenix/replication/metrics/MetricsReplicationLogDiscovery.java:
##########
@@ -62,6 +65,14 @@ public interface MetricsReplicationLogDiscovery extends
BaseSource {
*/
void updateTimeToProcessInProgressFiles(long timeMs);
+ /**
+ * Increments the counter for rounds whose new-file processing time exceeded
the round time. This
+ * counter lives on the shared discovery base, so it is emitted by both the
forwarder-side and
+ * replay-side discovery sources; a rising rate signals that that side's
new-file processing is
+ * falling behind the cadence at which new rounds become eligible.
Review Comment:
This explicitly exports `roundsExceedingRoundTime` from both discovery sides
because the counter and recording site are in the shared base; the new
rename-failure counter is likewise registered by the shared tracker base. The
PR describes all six metrics as standby replay-side metrics, so forwarder
MBeans now expose an undocumented extra surface. Move these
registrations/recording hooks into the replay-specific implementations, or
update the PR/API contract if forwarder metrics are intentional.
--
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]
