[CELEBORN-1984] Merge ResourceRequest to transportMessageProtobuf #3231
Conversation
|
@FMX plz help review |
Got it. I will add some unit tests to verify it. |
| @@ -454,6 +454,14 @@ message PbApplicationLostResponse { | |||
| int32 status = 1; | |||
| } | |||
|
|
|||
| message HeartbeatInfo { | |||
There was a problem hiding this comment.
Protobuf structures don't need to be defined before being used. All structures in the transport message have a common prefix "Pb". So I think this structure can be placed with all structures ported from the resources proto.
| @@ -906,3 +915,102 @@ message PbPushMergedDataSplitPartitionInfo { | |||
| message PbChunkOffsets { | |||
| repeated int64 chunkOffset = 1; | |||
| } | |||
|
|
|||
| enum Type { | |||
There was a problem hiding this comment.
Its name shall be PbMetaRequestType.
| ReviseLostShuffles = 29; | ||
| } | ||
|
|
||
| message ResourceRequest { |
There was a problem hiding this comment.
The structure name can follow the Pb prefix to avoid misunderstanding in the Java or Scala code base.
| optional PbReviseLostShuffles reviseLostShufflesRequest = 102; | ||
| } | ||
|
|
||
| message RequestSlotsRequest { |
| map<string, PbSlotInfo> workerAllocations = 3; | ||
| } | ||
|
|
||
| message AppHeartbeatRequest { |
| HeartbeatInfo heartbeatInfo = 7; | ||
| } | ||
|
|
||
| message WorkerExcludeRequest { |
| repeated WorkerAddress workersToRemove = 2; | ||
| } | ||
|
|
||
| message WorkerAddress { |
| int32 internalPort = 6; | ||
| } | ||
|
|
||
| enum Status { |
| INTERNAL_ERROR= 2; | ||
| } | ||
|
|
||
| message ResourceResponse { |
| @@ -91,6 +92,44 @@ public void testRunCommand() { | |||
| Assert.assertTrue(response.getSuccess()); | |||
| } | |||
|
|
|||
| @Test | |||
| public void testRunCommandByTransportMessage() { | |||
There was a problem hiding this comment.
You'll need to test the mixed protobuf message requests.
5590ef0 to
0dffcf6
Compare
What changes were proposed in this pull request?
as title
Why are the changes needed?
Merge Resource.proto into TransportMessages.proto as per the below design
https://cwiki.apache.org/confluence/display/CELEBORN/CIP-16+Merge+transport+proto+and+resource+proto+files
Does this PR introduce any user-facing change?
How was this patch tested?