devmadhuu commented on code in PR #11262:
URL: https://github.com/apache/ozone/pull/11262#discussion_r4085548000
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ReconTaskControllerImpl.java:
##########
@@ -502,6 +502,61 @@ private void processTasks(
} catch (CancellationException ce) {
LOG.error("Some tasks were cancelled with exception", ce);
}
+
+ failedTasks.addAll(executionFailedTasks);
+
+ if (!successfulTasks.isEmpty()) {
+ // Sync derived-data RocksDB WAL before committing task-status cursors
to avoid durability gaps.
+ boolean synced = syncReconDbLog();
Review Comment:
One failed fsync sets tasksFailed, which stops event buffering
(`consumeOMEvents`) and queues `TASK_FAILURES` `reinit` — a full staged
reprocess of all six tasks with a complete OM DB scan. That's a little too
harsh response to a transient I/O hiccup, and the flush isn't retried first.
Would a bounded retry on `flushLog` before escalating be worth it?
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/tasks/ReconTaskControllerImpl.java:
##########
@@ -502,6 +502,61 @@ private void processTasks(
} catch (CancellationException ce) {
LOG.error("Some tasks were cancelled with exception", ce);
}
+
+ failedTasks.addAll(executionFailedTasks);
+
+ if (!successfulTasks.isEmpty()) {
+ // Sync derived-data RocksDB WAL before committing task-status cursors
to avoid durability gaps.
+ boolean synced = syncReconDbLog();
+ for (ReconOmTask.TaskResult result : successfulTasks) {
+ String taskName = result.getTaskName();
+ ReconTaskStatusUpdater taskStatusUpdater =
+ taskStatusUpdaterManager.getTaskStatusUpdater(taskName);
+
+ if (synced) {
+ taskMetrics.incrTaskDeltaProcessingSuccess(taskName);
+ taskStatusUpdater.setLastTaskRunStatus(0);
+
taskStatusUpdater.setLastUpdatedSeqNumber(events.getLastSequenceNumber());
+ } else {
+ taskMetrics.incrTaskDeltaProcessingFailures(taskName);
+ taskStatusUpdater.setLastTaskRunStatus(-1);
+ }
+ taskStatusUpdater.recordRunCompletion();
+ }
+
+ if (!synced) {
+ // Signal task reinitialization directly instead of retrying process(),
+ // because task writes were already applied and retrying would
re-apply non-idempotent events.
+ tasksFailed.compareAndSet(false, true);
+ }
+ }
+ }
+
+ /**
+ * Flushes and syncs the Recon derived-data RocksDB write-ahead log to
stable storage so the
+ * derived writes for the processed batch are durable before the task-status
cursor advances.
+ *
+ * The cursor rows committed to Derby are fsync-durable while RocksDB writes
are not synced by default.
+ * Without this barrier, a power loss can leave the durable cursors ahead of
the (lost) derived data.
+ * On restart, reconciliation sees matching sequence numbers and never
reprocesses the events,
+ * permanently dropping the applied updates.
+ *
+ * @return {@code true} if sync succeeded; {@code false} if dbStore is null
or sync failed,
+ * in which case callers must not advance cursors and must signal
reinitialization.
+ */
+ private boolean syncReconDbLog() {
+ DBStore dbStore = reconDBProvider.getDbStore();
+ if (dbStore == null) {
+ LOG.error("Recon DB store is null; cannot sync WAL before advancing task
status cursor.");
+ return false;
+ }
+ try {
+ dbStore.flushLog(true);
Review Comment:
`flushLog(true) `has a side effect on failure. It delegates to
`RDBStore.flushLog (RDBStore.java)`, which calls `db.flushWal(sync) `on the
underlying `RocksDatabase`. That method calls `closeOnError(e)` before
rethrowing `(RocksDatabase.java)`, and `shouldClose` returns true for
Corruption and IOError. So on those codes recon.db's RocksDB handle is closed
by the time we catch the exception.
The reinit we signal does recover — it reprocesses into a staged DB and
replaceStagedDb reopens the live store But during that, Recon's read endpoints
will fail with RocksDatabaseException("Rocks Database is closed") and nothing
logs why. Could the catch block log that the store may now be closed? It would
save real debugging time.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
