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

PLAT-671: send VDelegatedRequestFinished after retries for correct pending count #568

Merged
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions ledger-core/virtual/execute/execute.go
Original file line number Diff line number Diff line change
Expand Up @@ -722,6 +722,8 @@ func (s *SMExecute) stepSendOutgoing(ctx smachine.ExecutionContext) smachine.Sta
}
} else {
if s.outgoingSentCounter >= MaxOutgoingSendCount {
// TODO when CallSummary will live longer than one pulse it needs to be updated
s.sendDelegatedRequestFinished(ctx, nil)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like the idea of setting "sendDelegatedRequestFinished" in every place where work can be stopped prematurely :)
I'd prefer to set "ErrorHandler"/"some kind of destructors" somewhere and catch all that situations at once.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We discuss this with @ruz and decided that this is important for pending tables. Or I can't understand your idea in second sentence.

Copy link
Contributor

@bigbes bigbes Jul 22, 2020

Choose a reason for hiding this comment

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

I mean that part of code (send pending request finished) must be executed on every exit scenario after we've received VDelegatedCallResponse

Copy link
Contributor

Choose a reason for hiding this comment

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

But that's a big addition, so I guess only if @ruz is OK

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, we need some kind of destructor for this.

return ctx.Error(throw.E("outgoing retries limit"))
}

Expand Down Expand Up @@ -916,6 +918,12 @@ func (s *SMExecute) stepSendDelegatedRequestFinished(ctx smachine.ExecutionConte
}
}

s.sendDelegatedRequestFinished(ctx, lastState)

return ctx.Stop()
}

func (s *SMExecute) sendDelegatedRequestFinished(ctx smachine.ExecutionContext, lastState *payload.ObjectState) {
msg := payload.VDelegatedRequestFinished{
CallType: s.Payload.CallType,
CallFlags: s.Payload.CallFlags,
Expand All @@ -934,10 +942,9 @@ func (s *SMExecute) stepSendDelegatedRequestFinished(ctx smachine.ExecutionConte
}
}
}).WithoutAutoWakeUp().Start()

return ctx.Stop()
}


func (s *SMExecute) makeNewDescriptor(class reference.Global, memory []byte) descriptor.Object {
var prevStateIDBytes []byte
objDescriptor := s.execution.ObjectDescriptor
Expand Down
1 change: 0 additions & 1 deletion ledger-core/virtual/execute/execute.plantuml
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,6 @@ state "stepSendDelegatedRequestFinished" as T01_S031
T01_S031 : SMExecute
T01_S031 --[dotted]> T01_S016
T01_S031 --[dotted]> T01_S001
T01_S031 --> T01_S011 : PrepareAsync(ctx).WithoutAutoWakeUp()
T01_S031 --> [*]
state "stepSendOutgoing" as T01_S025
T01_S025 : SMExecute
Expand Down
16 changes: 16 additions & 0 deletions ledger-core/virtual/handlers/vdelegatedrequestfinished.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,12 @@ type unexpectedVDelegateRequestFinished struct {
Ordered bool
}

type noLatestStateTolerableVDelegateRequestFinished struct {
*log.Msg `txt:"Tolerable VDelegateRequestFinished on Empty object has no LatestState"`
Object reference.Holder
Request reference.Holder
}

var dSMVDelegatedRequestFinishedInstance smachine.StateMachineDeclaration = &dSMVDelegatedRequestFinished{}

type dSMVDelegatedRequestFinished struct {
Expand Down Expand Up @@ -154,6 +160,16 @@ func (s *SMVDelegatedRequestFinished) updateSharedState(
if s.hasLatestState() {
state.SetDescriptorDirty(s.latestState())
s.updateObjectState(state)
} else {
if s.Payload.CallFlags.GetInterference() == contract.CallTolerable &&
(s.Payload.CallType == payload.CTMethod || s.Payload.CallType == payload.CTConstructor) &&
state.GetState() == object.Empty {
ctx.Log().Warn(noLatestStateTolerableVDelegateRequestFinished{
Object: objectRef,
Request: requestRef,
})
state.SetState(object.Missing)
}
}

pendingList := state.PendingTable.GetList(s.Payload.CallFlags.GetInterference())
Expand Down
4 changes: 4 additions & 0 deletions ledger-core/virtual/integration/contract_calls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -762,6 +762,8 @@ func TestVirtual_CallContractFromContract_RetryLimit(t *testing.T) {
return false
})

typedChecker.VDelegatedRequestFinished.Set(func(finished *payload.VDelegatedRequestFinished) bool { return false })

}

server.WaitIdleConveyor()
Expand All @@ -776,6 +778,8 @@ func TestVirtual_CallContractFromContract_RetryLimit(t *testing.T) {
testutils.WaitSignalsTimed(t, 10*time.Second, server.Journal.WaitAllAsyncCallsDone(), executeStopped, foundError)

require.Equal(t, countChangePulse, typedChecker.VCallRequest.Count())
require.Equal(t, countChangePulse, typedChecker.VDelegatedCallRequest.Count())
require.Equal(t, 1, typedChecker.VDelegatedRequestFinished.Count())

mc.Finish()

Expand Down