[Improvement][tez] Optimize the method of obtain the vertex id.

[zhengchenyu]

Code of Conduct

• I agree to follow this project's Code of Conduct

Search before asking

• I have searched in the issues and found no similar issues.

What would you like to be improved?

For now, vertex id is extract from vertex name. This way only support the vertex name like "Map 0", "Reduce 1", generally generated from hive.
For tez examples, the vertex name is arbitrary, so we can't get the vertex id. So we need a new way to get vertex id.

How should we improve?

No response

Are you willing to submit PR?

• Yes I am willing to submit a PR!

[lifeSo]

It is great, if can obtain vertex id throuth:

          int sourceVertexId = dag.getVertex(edge.getSourceVertexName()).getVertexId().getId();
          int destinationVertexId = dag.getVertex(edge.getDestinationVertexName()).getVertexId().getId();

The only one it modify lots of file, but it is ok.

but when could not get vertexid, eg:

int sourceVertexId = this.conf.getInt(RSS_SHUFFLE_SOURCE_VERTEX_ID, -1);

could we throw exception, then we could know something wrong, instead get -1 and wrong result.

[zhengchenyu]

It is great, if can obtain vertex id throuth:

          int sourceVertexId = dag.getVertex(edge.getSourceVertexName()).getVertexId().getId();
          int destinationVertexId = dag.getVertex(edge.getDestinationVertexName()).getVertexId().getId();

The only one it modify lots of file, but it is ok.

but when could not get vertexid, eg:

int sourceVertexId = this.conf.getInt(RSS_SHUFFLE_SOURCE_VERTEX_ID, -1);

could we throw exception, then we could know something wrong, instead get -1 and wrong result.

@lifeSo

assert is used to throw exception. You can see the below code.

    assert sourceVertexId != -1;
    assert destinationVertexId != -1;

And in fact, the vertex id must exists, so in general way, we will never throw exception.

[lifeSo]

sourceVertexId

Greate! @zhengchenyu zhengchenyu
How about the assert check is in computeShuffleId() method.
Then users who call this methods do not assert check explicitly.

[zhengchenyu]

@lifeSo

That's a good idea, as only computeShuffleId use vertex id.
But just like the code show below. I check whether vertex id is -1, but the -1 is the default value of config. If we want to move assert to computeShuffleId, I think getInt also should move to computeShuffleId. How about this way?

    int sourceVertexId = this.conf.getInt(RSS_SHUFFLE_SOURCE_VERTEX_ID, -1);
    int destinationVertexId = this.conf.getInt(RSS_SHUFFLE_DESTINATION_VERTEX_ID, -1);
    assert sourceVertexId != -1;
    assert destinationVertexId != -1;