Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SCB-300] acceptance test for @Compensable event timeout #173

Closed
wants to merge 1 commit into from

Conversation

lijasonvip
Copy link
Contributor

@lijasonvip lijasonvip commented Apr 16, 2018

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Make sure there is a JIRA issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a JIRA issue. Your pull request should address just this issue, without pulling in other changes.
  • Each commit in the pull request should have a meaningful subject line and body.
  • Format the pull request title like [SCB-XXX] Fixes bug in ApproximateQuantiles, where you replace SCB-XXX with the appropriate JIRA issue.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Run mvn clean install to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

Now we have acceptance test for @SagaStart event timeout in #171 , we need to test @Compensable event timeout too.

@coveralls
Copy link

coveralls commented Apr 16, 2018

Coverage Status

Coverage decreased (-0.4%) to 95.197% when pulling 521425f on lijasonvip:SCB-300 into be37cfe on apache:master.

METHOD postHotelBooking
AT ENTRY
IF TRUE
DO debug("delay 10s until the car booking timeout"),
Copy link
Member

Choose a reason for hiding this comment

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

Debug log is not right

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad.

| pack-hotel | TxAbortedEvent |
| pack-car | TxCompensatedEvent |
| pack-hotel | TxCompensatedEvent |
| pack-hotel | SagaEndedEvent |
Copy link
Member

Choose a reason for hiding this comment

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

I don't think pack-hotel should send the SagaEndedEvent here.

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'll fix this.


And Hotel Service contains the following booking orders
| name | amount | confirmed | cancelled |
| Jason | 1 | true | false |
Copy link
Member

Choose a reason for hiding this comment

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

The Hotel Service should be cancelled. There must be some errors in the Alpha Service.


Then Car Service contains the following booking orders
| name | amount | confirmed | cancelled |
| Jason | 1 | true | false |
Copy link
Member

Choose a reason for hiding this comment

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

The Car Service should be cancelled.

@WillemJiang
Copy link
Member

It looks like the Accept test result is not good. We need to fix the issue here.

@zhfeng
Copy link
Contributor

zhfeng commented Apr 17, 2018

what is the purpose of this test ? I think it would like to show the scenario that when the saga transaction is timeout, the alpha server call the compensate method is failed and will retry ?

The booking service looks like the following steps:

  1. saga start
  2. call the car booking
  3. call the hotel booking
  4. postBooking wait 10s until the transaction timeout (injected by the byteman rules)
  5. return failure message with the information of timeout ( I think it is still an issue when we are sending the SAGA_ENDED in the https://github.com/apache/incubator-servicecomb-saga/blob/master/omega/omega-transaction/src/main/java/org/apache/servicecomb/saga/omega/transaction/SagaStartAnnotationProcessor.java#L45). I think we should check the return value and throws the Exception if the transaction is timeout. Also the alpha server handles the message in (https://github.com/apache/incubator-servicecomb-saga/blob/master/alpha/alpha-core/src/main/java/org/apache/servicecomb/saga/alpha/core/TxConsistentService.java#L38), currently it only check if the transaction is aborted and I think it should also check if the transaction is timeout.

For this test, if we want to simulate the alpha server retries the compensate method when the transaction is timeout, I think it would like to use the byteman rule in the cancel method of HotelBooking (https://github.com/apache/incubator-servicecomb-saga/blob/master/saga-demo/booking/hotel/src/main/java/org/apache/servicecomb/saga/demo/pack/hotel/HotelBookingService.java#L40)

RULE create counter
CLASS org.apache.servicecomb.saga.demo.pack.hotel.HotelBooking
METHOD <init>
IF TRUE
DO createCountDown($0, 1)
ENDRULE

RULE throws the exception when calling the cancel method at the first time
CLASS org.apache.servicecomb.saga.demo.pack.hotel.HotelBookingService
MEHTOD cancel(HotelBooking)
AT ENTRY
IF !countDown($0)
DO throw exception
ENDRULE

@lijasonvip lijasonvip closed this May 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants