Aetherance commented on PR #3541: URL: https://github.com/apache/kvrocks/pull/3541#issuecomment-4858085094
I added optional output parameters to `String::Set` and `Database::MDel` so the command layer can emit notifications based on what actually changed. For `SET`, this avoids duplicating the conditional `SET` decision logic in the command layer. For `DEL`, this avoids an extra `EXISTS` pass before deletion, which would add unnecessary storage reads and overhead. It also lets `MDel` return the keys it actually deleted. The alternative would be to keep these storage APIs unchanged and infer the notification decisions in the command layer. That would keep the APIs smaller, but the trade-off is that `SET` would duplicate conditional `SET`/`GET` logic outside `String::Set`, and `DEL` would need extra reads before `MDel`. Over time, this could also drift from the actual write behavior. I chose the optional-output approach for this PR, but I’m open to maintainers’ preference here. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
