Skip to content
This repository has been archived by the owner on Nov 22, 2018. It is now read-only.

Handle unreliable caching #113

Closed
wants to merge 0 commits into from
Closed

Handle unreliable caching #113

wants to merge 0 commits into from

Conversation

JunTaoLuo
Copy link
Contributor

@JunTaoLuo JunTaoLuo commented May 18, 2016

#99 swallow exceptions caused by unreliable cache operations and log them. Will open issue on Caching for more reliable cache operations, retry logic ...

@@ -71,46 +71,77 @@ public class DistributedSession : ISession
_isNewSessionKey = isNewSessionKey;
}

public bool IsAvailable { private set; get; }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be moved to ISession in HttpAbstractions.

@JunTaoLuo
Copy link
Contributor Author

cc @Tratcher

}
_loaded = true;
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can remove TODO item on the like below since that's what we will go with. Same with the method Deserialize where we will not throw since it's self healing by creating a new session. ReadBytes may still fail due to corrupted cache but that exception will be caught in Load.

@Tratcher
Copy link
Member

Offline discussion: set a dummy store in the Load failure case so you don't have to check IsAvailable in every method.

@JunTaoLuo
Copy link
Contributor Author

🆙📅

catch (Exception exception)
{
_logger.SessionCacheReadException(_sessionKey, exception);
return;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't return here, this isn't fatal. Try the commit and let that fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That works but you get two exceptions instead of one logged. I guess the duplication there isn't a problem.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Depending on how transient the error is, commit may succeed.

@JunTaoLuo
Copy link
Contributor Author

🆙📅

@Tratcher
Copy link
Member

:shipit:

@JunTaoLuo JunTaoLuo closed this May 23, 2016
@JunTaoLuo JunTaoLuo deleted the johluo/unreliable-storage branch May 23, 2016 17:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants