uDiscovery redesign #242
Open
uDiscovery redesign #242
Conversation
This file contains 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
The following PR is a reboot of uDiscovery to be able to take all the lessons learned from existing implementations in production. eclipse-uprotocol#137
sophokles73
requested changes
Sep 26, 2024
| @@ -1,5 +1,5 @@ | |||
| /* | |||
| * SPDX-FileCopyrightText: 2023 Contributors to the Eclipse Foundation | |||
| * SPDX-FileCopyrightText: 2024 Contributors to the Eclipse Foundation | |||
There was a problem hiding this comment.
this does not need to be changed, given that it is an existing file and unchanged major version ...
Comment on lines
+30
to
+31
| // addresses (in URI format), and properties of topics. This API is used by clients (uEs) to be | ||
| // about to access the information within the database. Clients talk to their local uDiscovery service |
There was a problem hiding this comment.
Suggested change
| // addresses (in URI format), and properties of topics. This API is used by clients (uEs) to be | |
| // about to access the information within the database. Clients talk to their local uDiscovery service | |
| // addresses (in URI format), and properties of topics. This API is used by clients (uEs) to | |
| // access the information within the database. Clients talk to their local uDiscovery service |
| // The uDiscovery service is used to store information about devices and services namely their | ||
| // addresses (in URI format), and properties of topics. This API is used by clients (uEs) to be | ||
| // about to access the information within the database. Clients talk to their local uDiscovery service | ||
| // who, when is unable to find what it is looking for, queries the central database to find the information. |
There was a problem hiding this comment.
Suggested change
| // who, when is unable to find what it is looking for, queries the central database to find the information. | |
| // which, if unable to find what it is looking for, queries the central database to find the information. |
| // addresses (in URI format), and properties of topics. This API is used by clients (uEs) to be | ||
| // about to access the information within the database. Clients talk to their local uDiscovery service | ||
| // who, when is unable to find what it is looking for, queries the central database to find the information. | ||
| // UDiscovery works in heiarchial mode where the local uDiscovery service is the first point of contact |
There was a problem hiding this comment.
Suggested change
| // UDiscovery works in heiarchial mode where the local uDiscovery service is the first point of contact | |
| // The UDiscovery service instances form a hierarchy where the local uDiscovery service is the first point of contact |
| rpc LookupUri(LookupUriRequest) returns (LookupUriResponse) { | ||
| // Find Services. | ||
| // | ||
| // Discover t authorities, instances, and versions of services based on the passed URI. |
There was a problem hiding this comment.
Suggested change
| // Discover t authorities, instances, and versions of services based on the passed URI. | |
| // Discover authorities, instances, and versions of services based on the passed URI. |
|
|
||
| [.specitem,oft-sid="dsn~discovery-data-reconciliation-data~1",oft-needs="impl,test"] | ||
| -- | ||
| * When reconciling data, the data fetched *MUST* be used in lieu of current cached data. |
There was a problem hiding this comment.
Suggested change
| * When reconciling data, the data fetched *MUST* be used in lieu of current cached data. | |
| * When reconciling data, a client *MUST* replace corresponding data in its local cache with the fetched data. |
|
|
||
| [.specitem,oft-sid="dsn~discovery-data-reconciliation-not-found~1",oft-needs="impl,test"] | ||
| -- | ||
| * If `GetServiceTopics()` returns `UCode.NOT_FOUND` for data we had in our local instance, we *MUST* flush the local cache and then replicate the deletion. we fetch information to reconcile the data If the reconciliation fails, the service *MUST* log the error and continue to use the current cached data. |
There was a problem hiding this comment.
I am not sure who is we in these sentences. Can you rephrase this being (super-)explicit about the roles?
|
|
||
|
|
||
| === SetServiceTopics() | ||
| The `SetServiceTopics()` API is used by the uDiscovery business logic to add, update, or remove information recursively between the local, domain, and central instances. The API is passed `SetServiceTopicsRequest` containing a repeated list of `UServiceTopicInfo` and `ttl` for when the info will expire. The function returns `SetServiceTopicsResponse` message if the method was successfully processed or it returns `UStatus` containing the error values specified below. |
There was a problem hiding this comment.
Where does the information come from originally, i.e. who is registering the info in the local uDiscovery service? Is it the uEntities?
|
|
||
| [.specitem,oft-sid="dsn~discovery-data-reconciliation-not-found~1",oft-needs="impl,test"] | ||
| -- | ||
| * If `GetServiceTopics()` returns `UCode.NOT_FOUND` for data we had in our local instance, we *MUST* flush the local cache and then replicate the deletion. we fetch information to reconcile the data If the reconciliation fails, the service *MUST* log the error and continue to use the current cached data. |
There was a problem hiding this comment.
is this the FindServiceTopics() operation described above in the client API section?
| participant Domain-UDiscovery | ||
| participant Central-UDiscovery | ||
|
|
||
| Domain-UDiscovery ->> Local-UDiscovery: GetServiceTopics() |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The following PR is a reboot of uDiscovery to be able to take all the lessons learned from existing implementations in production.
#137