Make workerinfo stored as json#17959
Merged
alluxio-bot merged 12 commits intoAlluxio:mainfrom Aug 18, 2023
Merged
Conversation
2. remove redundant lock in checkAllForReconnect and add elaboration on mNeedReconnect instance var
threadpool is not allowed to be modified 2. checkstyle fixes
…ass AlluxioEtcdClient
Kai-Zhang reviewed Aug 10, 2023
| public void deserialize(DataInputStream dis) throws IOException { | ||
| mServiceEntityName = dis.readUTF(); | ||
| mRevision = dis.readLong(); | ||
| public static String toJson(DefaultServiceEntity entity) { |
Contributor
There was a problem hiding this comment.
maybe leave the implement to decide the serialization type? license uses the service entity and it won't use json for now. maybe we can provide interfaces like this?
public static byte[] serialize(DefaultServiceEntity entity) throws IOException {...} public static DefaultServiceEntity deserialize(byte[] buf) throws IOException {...}| * @param clusterIdentifier | ||
| */ | ||
| public ServiceDiscoveryRecipe(AlluxioEtcdClient client, String clusterIdentifier) { | ||
| ServiceDiscoveryRecipe(AlluxioEtcdClient client, String clusterIdentifier) { |
Contributor
There was a problem hiding this comment.
license engine may need to construct its own ServiceDiscoveryRecipe. is there any context on this change?
LuQQiu approved these changes Aug 16, 2023
Contributor
LuQQiu left a comment
LuQQiu left a comment There was a problem hiding this comment.
LGTM, thanks for supporting json worker info!
lucyge2022 added the type-ease-of-use Contributor Author
| alluxio-bot, merge this please. |
alluxio-bot pushed a commit that referenced this pull request Aug 22, 2023
### What changes are proposed in this pull request? - Change the serialize/deserialize interface for future use(#17959) - Expose the TTL and timeout of the lease used in `ServiceDiscoveryRecipe` - Expose the path prefix of the `ServiceDiscoveryRecipe` for future use ### Why are the changes needed? etcd will be used in the near future for more areas than worker service discovery. so expose more information for future use. ### Does this PR introduce any user facing changes? nope pr-link: #18010 change-id: cid-7cc0b9f5cfaee860e587941c37867797b7fe7bdb
Contributor
| any particular consideration for choosing Gson over Jackson? |
Contributor Author
actually no, i saw Gson was used in codebase and has much simpler syntax. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes are proposed in this pull request?
Store workerinfo as json format on etcd for EtcdMembershipManager.
Why are the changes needed?
For cross-language worker membership retrieval.
Does this PR introduce any user facing changes?
No.