Remove the catalyst dependencies in backup transport #12056
Conversation
|
Automated checks report:
Some checks failed. Please fix the reported issues and reply 'alluxio-bot, check this please' to re-run checks. |
|
Merged build finished. Test FAILed. |
|
Test FAILed. |
|
Merged build finished. Test FAILed. |
|
Test FAILed. |
|
06:05:08.418 [ERROR] alluxio.master.journal.raft.RaftJournalTest.gainPrimacyAfterCatchup |
|
Jenkins, test this please |
|
Merged build finished. Test FAILed. |
|
Test FAILed. |
|
17:39:17.802 [ERROR] alluxio.server.ft.journal.raft.EmbeddedJournalIntegrationTest.restartStress Time elapsed: 120.198 s <<< ERROR! |
|
Jenkins, test this please |
|
Merged build finished. Test FAILed. |
|
Test FAILed. |
|
Merged build finished. Test FAILed. |
|
Test FAILed. |
|
Merged build finished. Test PASSed. |
|
Test PASSed. |
|
Automated checks report:
All checks passed! |
| * | ||
| * @param <T> the listener type | ||
| */ | ||
| public class Listeners<T> implements Iterable<Listener<T>> { |
There was a problem hiding this comment.
Is there any Java built-in or guava alternative of this listener we can use?
There was a problem hiding this comment.
The current implementation bounds the listeners with a shared thread context. So that all the listeners accept method will be executed by the same executor.
| * The context for Grpc messaging single thread. | ||
| * This context uses a {@link ScheduledExecutorService} to schedule events on the context thread. | ||
| */ | ||
| public class GrpcMessagingContext { |
There was a problem hiding this comment.
Is this class needed? Will there be any problem running everything without this context?
There was a problem hiding this comment.
This class mainly store two things, one executor, and one serializer.
In all the places, the current code makes sure that we read the response and write request to the same serializer.
all the requests are handled by the same executor, all the listeners acceptances are handled by the same executor.
It's more for the correctness and reduces the potential race conditions.
There was a problem hiding this comment.
I took a deeper look into the code, the backup leader may create more than one bi-di stream (in case need to reconnect?) with the client, but all the streams shared the same context (same executor and serializer).
There was a problem hiding this comment.
I see. We can leave it as it for now.
For serializer we will probably replace it with protobuf built-in serialization. The gRPC also provide some guarantee in message order, so this class may eventually become unnecessary.
|
alluxio-bot, merge this please |
No description provided.