-
Notifications
You must be signed in to change notification settings - Fork 1.2k
SONiC Mgmt support for Multiple DB extensions #2089
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
SONiC Mgmt support for Multiple DB extensions #2089
Conversation
|
/azp run |
|
|
No pipelines are associated with this pull request. |
|
/azp run |
|
No pipelines are associated with this pull request. |
| 8. DB access layer performs a WATCH of all the keys in the Redis DB. | ||
| 9. Status being returned from Redis. | ||
| 10. Status being returned from DB access layer. | ||
| 11. Translib then invokes the prepareUpdate() API on the App module. For Update, Replace, or Delete operations keys are constructed based on the original request keys and existing row entry of corresponding key is fetched from database and the following (table_name, key, row entry, operation) are written in a backup file based on namespace. For Create, only the key and table_name and operation is written in the backup file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Additional responsibility for all app modules
- Additional query overhead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be overhead Multi db involvement. For single db this overhead will not be applicable.
| 2. Non-commited DB Abort: DB access layer first invokes UNWATCH request on the Redis DB indicating abort everything together. | ||
| 3. Non-commited DB Abort: Status returned from Redis. | ||
| 4. Non-commited DB Abort: Status returned from DB access layer | ||
| 5. Commited DB Rollback: Revert modified entry by reading backup data from backup file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will not work..
-
It creates a churn in the system hw/sw state and can lead to unexpected traffic behavior.
The EXEC at step 17.6 would have resulted in redis notifications to all interested backends and they would have already started acting on it. -
Since the WATCH scope ends after EXEC, someone could have updated the same entries. This rollback will overwrite them too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Yes currently incase of rare failure during commit the rollback will lead to redis notifications. Can you suggest if there are any other options to avoid these notifications with multi db support.
- Even though WATCH scope ends after EXEC write lock in translib framework (sonic-mgmt-common) will not allow till the current execution completes.
anders-nexthop
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting. Does the namespace processing add any significant overhead? If so, to what extent would the single-namespace case be affected?
| The gNMI support requires the gNMI server which is provided as a part of sonic-telemetry container. We would like to rename this container as the sonic-gnmi container as now it can perform configurations as well through the gNMI server. | ||
| Will introduce a new container sonic-mgmt-common to host the common code that is used both in the mgmt-framework and sonic-telemetry container. This new repo will compile into static libraries that will be used in the other two repos. This way sonic-telemetry repo can be compiled without the mgmt-framework being present in the code base. | ||
| The gNMI support requires the gNMI server which is provided as a part of sonic-gnmi container. | ||
| A new repo sonic-mgmt-common to host the common code that is used both in the mgmt-framework and sonic-gnmi container. This new repo will compile into static libraries that will be used in the other two repos. This way sonic-gnmi repo can be compiled without the mgmt-framework being present in the code base. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: "A new repo sonic-mgmt-common will host..." rather than "A new repo sonic-mgmt-common to host", it reads better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes we will address this comment
|
SONiC Mgmt support for Multiple DB extensions