Github user arunmahadevan commented on the pull request:
https://github.com/apache/storm/pull/664#issuecomment-131602837
@harshach the changes I made were in the trident implementation (HDFSState)
which is independent of this. Anyways I reviewed the changes.
Overall it appears that the exceptions are now handled individually at
write, sync and rotate phases. But I see a few issues with the change.
- the tuples are acked only on sync - if the tuple rates are low, they will
never be acked and keep timing out and same tuples will be replayed again and
again.
- there is an attempt to sync even when the write fails with an
IOException. Since write already threw an IOException, chances are high that
the sync would also fail with IOException.
I think it might be simpler to keep the existing logic and just rotate the
file whenever we see an IOException (or maybe after a few times we repeatedly
hit the IOException) and completely fail by propagating the exception up if the
situation does not improve after a few rotations.
Also I see that the existing implementation acks the tuples before actually
syncing them to disk, which might result in data loss. I think we should change
this to ack only after an hsync and have a sync policy that considers both
count and time based thresholds.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
