[server][da-vinci-client] Mark a helix replica to error state during ingestion error #1337
Conversation
majisourav99
force-pushed
the
helixError
branch
from
287ec66
to
038191f
Compare
|
Can we ensure that not all replicas for the current version are marked as ERROR, so we can still serve traffic with some replicas (even if it's stale) |
Discussed offline, this PR will reset the replica when we are marking CV to error as well, so there will not be any extra risk to read path not having any replica to server reads. |
majisourav99
force-pushed
the
helixError
branch
from
3330617
to
2ab455a
Compare
| @@ -79,7 +79,7 @@ public class HelixParticipationService extends AbstractVeniceService | |||
| private final String clusterName; | |||
| private final String participantName; | |||
| private final String zkAddress; | |||
| private final StoreIngestionService ingestionService; | |||
| private StoreIngestionService ingestionService = null; | |||
| @@ -525,6 +533,14 @@ public StoreIngestionTask( | |||
| } | |||
| this.batchReportIncPushStatusEnabled = !isDaVinciClient && serverConfig.getBatchReportEOIPEnabled(); | |||
| this.parallelProcessingThreadPool = builder.getAAWCWorkLoadProcessingThreadPool(); | |||
| this.zkHelixAdmin = Lazy.of(() -> new ZKHelixAdmin(zkAddress)); | |||
There was a problem hiding this comment.
This seems to be very expensive to have one ZKHelixAdmin per SIT, I think we should use a shared ZKHelixAdmin among all the SITs.
I bet each ZKHelixAdmin instance will create one ZK client internally.
| // Set the replica state to ERROR so that the controller can attempt to reset the partition. | ||
| if (!isDaVinciClient) { | ||
| zkHelixAdmin.get() | ||
| .setPartitionsToError( |
There was a problem hiding this comment.
This is to set EV state as ERROR, right?
There was a problem hiding this comment.
I saw you mentioned that this logic would only mark EV state as ERROR when CV is in ERROR state, so how can we achieve that by the current logic?
Only the following logic today decides whether to propagate ERROR to CV/EV IIUC.
Mark a helix replica to error state during ingestion error
If ingestion task encounters any error, it just marks the CV to error state which never remediates. This PR makes user of a new API helix added to annotate a replica to ERROR state. This will later be picked by the error replica reset task
ErrorPartitionResetTaskwhich will attempt to recover those replicas.How was this PR tested?
GHCI
Does this PR introduce any user-facing changes?