galenwarren commented on a change in pull request #303:
URL: https://github.com/apache/flink-statefun/pull/303#discussion_r813160085
##########
File path: statefun-sdk-go/v3/pkg/statefun/context.go
##########
@@ -76,6 +81,12 @@ func (s *statefunContext) Storage() AddressScopedStorage {
return s.storage
}
+func (s *statefunContext) WithContext(ctx context.Context) Context {
+ newContext := *s
+ newContext.Context = ctx
+ return &newContext
Review comment:
Ah, I understand what you're saying.
I think that, with regard to the mutex and the response, we need the old and
new contexts to share the same state, because both contexts need to update the
same response object, i.e. the one (and only) response object that will be
returned from the invocation of the stateful function.
I think that also holds true for the storage; there is only one storage
instance that applies for the entire stateful-function call, so both contexts
need to reference it. (And the storage operations are protected with a
sync.RWMutex, so this should be safe.)
So that just leaves the caller. My understanding is that this is a pointer
in order that it can be nil, to handle the case where there is no caller, but
that it is also intended to be read-only. Even without the addition of
`WithContext`, two consumers of the same `statefun.Context` could conceivably
step on each others' toes by modifying the `Address` returned from `Caller`. I
think the assumption is that the returned `Address` will be read-only, and with
that assumption, we wouldn't be making anything worse by shallow copying vs.
deep copying.
But, deep copying the caller `Address` would certainly be possible.
@sjwiesman @tillrohrmann Do you have any thoughts on this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
