Initial Request Failure on Retry
Locked
 
|
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.
We tracked the issue down to the WAPathConsumer, which keeps track of the path (destructively) as the request is being resolved. Its state isn't being reset before the request is retried, so subsequent requests are effectively tried against a different path. We've fixed in our home-grown Zinc streaming adaptor by adding the following line just before we retry: > aRequestContext consumer initializeWith: aRequestContext request url path copy This is admittedly a dreadful encapsulation violation. Ideally, maybe we'd implement something like #reset on WARequestContext. There is some other internal state (like `handlers`) that should probably be reset too, though I think in practice the use of #ensure: in #push:during: empties out the handlers collection before handling is retried. Alternately, we could perhaps make WAPathConsumer non-destructive and implement #reset there. It already acts a bit like a stream, so a #reset method would not be too out of place there. Thoughts? -- Ken Treis Miriam Technologies, Inc. |
Re: [Seaside-dev] Initial Request Failure on Retry
|
|
On Thu, Mar 14, 2013 at 7:12 PM, Ken Treis <[hidden email]> 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? 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. Cheers Philippe |
|
|
On Mar 16, 2013, at 2:05 AM, Philippe Marschall wrote:
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.
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... Ken |
Re: [Seaside-dev] Initial Request Failure on Retry
|
|
On Wed, Mar 20, 2013 at 7:30 AM, Ken Treis <[hidden email]> wrote:
> On Mar 16, 2013, at 2:05 AM, Philippe Marschall wrote: > > On Thu, Mar 14, 2013 at 7:12 PM, Ken Treis <[hidden email]> 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 |
«
Return to GLASS
|
1 view|%1 views
| Free forum by Nabble | Edit this page |