compensation method invoke before compensable method

[stashslash]

i test the saga-spring-demo

if i modify the HotelBookingService add some sleep code, like:

@Compensable(compensationMethod = "cancel", timeout = 5)
  void order(HotelBooking booking) {
    if (booking.getAmount() > 2) {
      throw new IllegalArgumentException("can not order the rooms large than two");
    }
    booking.confirm();

   // sleep 10 sec, simulate slow db
    try {
      Thread.sleep(10 * 1000);
    } catch (InterruptedException e) {
      e.printStackTrace();
    }

    bookings.put(booking.getId(), booking);
  }

then, i call the booking rest api, and got the booking result from hotel service and car service, the result looks like:

• hotel service:
  [{"name":"test","amount":2,"confirmed":true,"cancelled":false}]
• car service:
  [{"name":"test","amount":2,"confirmed":false,"cancelled":true}]

it's looks like car service compensated success, but hotel service compensated failure.

so, did i missing anything?

[zhfeng]

It looks like the timeout happens during the HotelBookingService is sleeping ? And the compensate method was invoked before the booking is finished. So it maybe need to add a lock to check if the transaction is cancelled.

void order(HotelBooking booking) {
...
    locker.lock();
    if (! isCancelled) {
         doBooking();
    } else {
         throw Exception("transaction is cancelled");
    }
    locker.unlock();
}

void cancel(HotelBooking booking) {
     locker.lock();
     isCancelled = true;
     doCancel();
     locker.unlock();
}

[stashslash]

@zhfeng thx for reply.
like you said, and i know add a lock should works.
but my confusion is if i should add a lock check on each compensate method ? or it has a better way to solve that, like servicecmob-saga help me do it.

[zhfeng]

OK, I see. The alpha server needs to check the TxEndedEvent is received before doing the compensate.
would you mind to raise a JIRA for this user case ?

Thanks @stashslash

[WillemJiang]

As it is caused by the timeout, Alpha don't have chance to know if the compensable method is finished or not. Alpha have to abort the transaction by invoke the compensation method.

I don't think a lock could resolve this issue perfectly, because of the timeout could be caused by lot of reason. Such as the if the network connection is broken, Alpha cannot receive the TxEndedEvent, but the compensable method is finished.

I'm think about let the Omega take control of timeout checking, it could be much better for us to handle the timeout situation.

[stashslash]

it sounds great! and i raised SCB-1057

[zhfeng]

@WillemJiang I am not sure how the Omega can handle this timeout situation ? The Alpha need to guarantee that the compensate method should be invoked after the local transaction is finished, is it right ?

[WillemJiang]

@zhfeng Alpha cannot know that, how about LRA? Does the Coordinator know if the action is timeout.

[zhfeng]

@WillemJiang I think the current Narayana LRA implements the timeout at the coordinate side.

1. set a schedule to cancel the transaction when enlist the participant in LRARecord.
2. doEnd in the LRA Transaction when timeout happens.

So it maybe has the similar situation that the compensate method could be invoked before the business is done. And I think it is not clear described at the LRA spec timeout.

I think we can bring this case to the LRA issues.

[zhfeng]

@stashslash I proposal to update the booking demo at this timeout situation. Can you check this could resolve your problem ?

As we discussed, the coordinator cannot guarantee that the compensate method will be invoked after the business is done.

[WillemJiang]

@zhfeng My question is what does doEnd do when the timeout happens. If the Omega can cancel the invocation that could be great.

[zhfeng]

@WillemJiang it cancels the transaction when timeout happens and this is done at the coordinator side.

[zhfeng]

@stashslash @WillemJiang it looks like we need to indicate in the cancel method that the compensate is ongoing. Now I think it might throw the Exception which could cause the alpha server to re-invoke this compensate method later.

void cancel(HotelBooking booking) throws Exception {
    Integer id = booking.getId();
    if (bookings.containsKey(id)) {
      bookings.get(id).cancel();
    } else {
      throw new Exception("can not cancel " + booking.getId());
    }
}

Also we could consider to add the status interface which reports to the alpha server.

[WillemJiang]

+1 for adding the status interface. BTW, I think we can let the omega check the type of exception to decide if it need to retry it again.

[stashslash]

@zhfeng if Omega could handle status，we just need bookings.get(id).cancel(); in compensate method, right？