Sumit6307 commented on PR #7981:
URL: https://github.com/apache/incubator-seata/pull/7981#issuecomment-4286004615
> Hi @Sumit6307, two issues after testing locally:
>
> 1. `super.unregisterResource(...)` in `DataSourceManager` throws
`NotSupportYetException` (parent still throws). `close()` fails on
> every call, `TableMetaCacheFactory.shutdown` never runs. Drop the super
call, or delegate to
> `RmNettyRemotingClient.getInstance().unregisterResource(group,
resourceId)` from #8020.
> 2. The test mocks `DefaultResourceManager.get()`, so the modified
`DataSourceManager.unregisterResource()` never executes — that's
> why CI passes. Please add a test without mocking
`DefaultResourceManager` that asserts `close()` doesn't throw and the cache
entry
> is removed.
>
> Also suggest try/finally in `close()` so `TableMetaCacheFactory.shutdown`
runs even if unregister fails.
Hi @WangzJi and @funky-eyes,
I've updated the PR to address these specific issues:
1. **Dropped `super.unregisterResource`**:
In `DataSourceManager`, I removed the
`super.unregisterResource(dataSourceProxy)` call so it no longer throws
`NotSupportYetException`. Instead, it now just removes the resource from the
`dataSourceCache` and delegates directly to
`RmNettyRemotingClient.getInstance().unregisterResource()`.
2. **Added `try/finally` block in `DataSourceProxy.close()`**:
I wrapped the unregistration inside a `try/finally` block. Now,
`TableMetaCacheFactory.shutdown(resourceId)` is guaranteed to run even if the
`unregisterResource` call fails or throws an exception.
3. **Added Real Integration Test**:
I added a new test `testCloseRemovesResourceFromManager` in
`DataSourceProxyTest` that creates a real `DruidDataSource` and
`DataSourceProxy` without mocking `DefaultResourceManager.get()`. It asserts
that the proxy is properly removed from the system's active managed resources
cache without throwing any exceptions during `close()`.
--
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]
