funky-eyes commented on code in PR #7981:
URL: https://github.com/apache/incubator-seata/pull/7981#discussion_r2945594924
##########
rm-datasource/src/main/java/org/apache/seata/rm/datasource/DataSourceManager.java:
##########
@@ -93,7 +93,9 @@ public void registerResource(Resource resource) {
@Override
public void unregisterResource(Resource resource) {
- throw new NotSupportYetException("unregister a resource");
+ DataSourceProxy dataSourceProxy = (DataSourceProxy) resource;
+ dataSourceCache.remove(dataSourceProxy.getResourceId());
+ super.unregisterResource(dataSourceProxy);
Review Comment:
```suggestion
super.unregisterResource(dataSourceProxy);
DataSourceProxy dataSourceProxy = (DataSourceProxy) resource;
dataSourceCache.remove(dataSourceProxy.getResourceId());
```
I believe we should first deregister the resource and then remove it locally
to avoid a situation where the server continues sending operations for a
resource that has already been deleted on the client. Additionally, I’m
wondering if it might be necessary to introduce a delay (e.g., sleep for a
while) to account for potential network issues, ensuring that any requests
still needing the resource have time to complete. This is because the
deregistration request sent to the server and the server’s subsequent
instructions sent to the client are processed in parallel. Even though the
resource has been deregistered on the server, requests already in progress
before the deregistration might still be sent.
##########
core/src/main/java/org/apache/seata/core/rpc/netty/RmNettyRemotingClient.java:
##########
@@ -275,6 +277,46 @@ public void registerResource(String resourceGroupId,
String resourceId) {
}
}
+ /**
+ * Unregister resource.
+ *
+ * @param resourceGroupId the resource group id
+ * @param resourceId the resource id
+ */
+ public void unRegisterResource(String resourceGroupId, String resourceId) {
+ if (StringUtils.isBlank(transactionServiceGroup) ||
StringUtils.isBlank(resourceId)) {
+ return;
+ }
+
+ if (getClientChannelManager().getChannels().isEmpty()) {
+ return;
+ }
+
+ synchronized (getClientChannelManager().getChannels()) {
+ for (Map.Entry<String, Channel> entry :
getClientChannelManager().getChannels().entrySet()) {
+ Channel rmChannel = entry.getValue();
+ String serverVersion =
getClientChannelManager().getChannelVersion(rmChannel);
+ // Threshold for version-aware unregistration: version >= 2.1.0
Review Comment:
Why is it version 2.1.0 or above, rather than 2.7.0 or above?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
