[Seaside-dev] Initial Request Failure on Retry

[Philippe Marschall]

On Wed, Mar 20, 2013 at 7:30 AM, Ken Treis <ken at miriamtech.com> wrote:
> On Mar 16, 2013, at 2:05 AM, Philippe Marschall wrote:
>
> On Thu, Mar 14, 2013 at 7:12 PM, Ken Treis <ken at miriamtech.com> wrote:
>
> We have build something using the Seaside-Rest add-on, and everything works
> beautifully in GemStone until a transaction conflict triggers a retry -- at
> which point we get a 404 rather than the expected response.
>
>
> Can you provide a bit more detail about the retry code? Where (server
> adapter,filter, ...) does it run and what does it look like?
>
>
> It's in a server adaptor that was stitched together from Dale's standard
> code for GemStone, a streaming hack for Zinc, and a quick back-port of your
> WAComboResponse for use in Seaside 3.0. Code is available at
> http://mc.miriamtech.com/zinc
>
> The starting point is WAGsZincStreamingAdaptor>>handleRequest:
> aRequestContext
>
> 1. Spins until we get necessary GS locks.
> 2. Attempts to process the request
> 3. Tries to commit the GS transaction.
> 4. If commit fails, abort and attempt to process again.
>
> Details below. This is really a GemStone-specific issue, but my life would
> be simpler with something like #reset or #resetIfPossible on WAPathConsumer
> and/or WARequestContext.
>
> Thinking about it you may actually need a bit more than just resetting
> the path consumer. The properties on the request context and the
> response buffer as well.
>
>
> I'm resetting the response buffer by sending #resetIfPossible, and we've
> hacked the path consumer reset into there, but I'm not doing anything with
> properties on the request context. What might need to be changed there?
>
> Here's a quick overview of the retry-related code in
> WAGsZincStreamingAdaptor, with some GemStone-specific details left out or
> pseudo-coded for brevity.
>
> handleRequest: aRequestContext
> | timeout |
> timeout := Time now addSeconds: 60.
> [self handleRequestCheckingLock: aRequestContext]
> whileFalse: [
> Time now > timeout
> ifTrue: [
> |response|
> response := aRequestContext response.
> response internalError.
> response stream nextPutAll: 'The system is too busy, please try again in a
> moment.'.
> ^self
> ].
> (Delay forMilliseconds: 10) wait.
> aRequestContext consumer initializeWith: aRequestContext request url path
> copy ]
>
> (I know, the path consumer reset feels out of place here... this was just a
> quick hack to make things work.)
>
> handleRequestCheckingLock: aRequestContext
> "answer false to retry request"
>
> | result retryRequest |
> GRPlatform current transactionMutex critical: [
> retryRequest := false.
> [ super handleRequest: aRequestContext ]
> on: WARetryHttpRequest
> do: [:ex | retryRequest := aRequestContext response isCommitted not ].
> retryRequest
> ifTrue: [
> "lock not acquired - unwind the stack to this point and leave transaction
> mode"
>
> GRPlatform current doAbortTransaction.
> ^false ].
> GRPlatform current doCommitTransaction
> ifFalse: [
> GRPlatform current doAbortTransaction.
> aRequestContext response resetIfPossible
> ifTrue: [
> GRPlatform current saveLogEntry: 'Commit failure - retrying'.
> ^false]
> ifFalse: [
> GRPlatform current saveLogEntry: 'Commit failure after response sent, giving
> up'.
> ^true]].
> ].
> ^true
>
> My apologies for the poor code factoring/formatting.
>
> Julian suggested creating a new context, which is something I hadn't
> considered but probably should...

Yes, this sounds like a better approach.

Cheers
Philippe