-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
Pass in actor creation dummy object for Java #3437
Conversation
@@ -172,7 +172,7 @@ private static ByteBuffer convertTaskSpecToFlatbuffer(TaskSpec task) { | |||
final int parentTaskIdOffset = fbb.createString(task.parentTaskId.toByteBuffer()); | |||
final int parentCounter = task.parentCounter; | |||
final int actorCreateIdOffset = fbb.createString(task.actorCreationId.toByteBuffer()); | |||
final int actorCreateDummyIdOffset = fbb.createString(UniqueId.NIL.toByteBuffer()); | |||
final int actorCreateDummyIdOffset = fbb.createString(task.actorId.toByteBuffer()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dummy object ID isn't actually the actor ID, right? It's the dummy object for the actor creation task.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the Python frontend, that's true, but it turns out that Java is actually using the dummy object ID as the actor ID here. I think it's actually nice to have some association like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see, thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing I found useful about having this association is when I debug issues about actor reconstruction, I can easily find the actor creation task id from the actor id, and vice versa.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -172,7 +172,7 @@ private static ByteBuffer convertTaskSpecToFlatbuffer(TaskSpec task) { | |||
final int parentTaskIdOffset = fbb.createString(task.parentTaskId.toByteBuffer()); | |||
final int parentCounter = task.parentCounter; | |||
final int actorCreateIdOffset = fbb.createString(task.actorCreationId.toByteBuffer()); | |||
final int actorCreateDummyIdOffset = fbb.createString(UniqueId.NIL.toByteBuffer()); | |||
final int actorCreateDummyIdOffset = fbb.createString(task.actorId.toByteBuffer()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing I found useful about having this association is when I debug issues about actor reconstruction, I can easily find the actor creation task id from the actor id, and vice versa.
Test FAILed. |
Some tests failing, but the Java ones look good. |
What do these changes do?
Fixes #3434.
Also, Java has a nicer way of handling this in that the object ID returned by an actor creation task is automatically used as the actor ID. We should consider doing the same in Python.