korlov42 commented on code in PR #6932:
URL: https://github.com/apache/ignite-3/pull/6932#discussion_r2513070573
##########
modules/client/src/main/java/org/apache/ignite/internal/client/sql/ClientSql.java:
##########
@@ -365,10 +366,18 @@ private <T> PayloadReader<AsyncResultSet<T>>
payloadReader(
assert table != null;
+ PaCacheKey key = new PaCacheKey(statement);
mappingProviderCache.put(
- new PaCacheKey(statement),
+ key,
PartitionMappingProvider.create(
- table, partitionAwarenessMetadata
+ table,
+ partitionAwarenessMetadata,
+ th -> {
+ if (th instanceof TableNotFoundException) {
Review Comment:
I thought about adding logging, but decided not to.
Let's split exceptions in two groups: expected and not expected.
One example of expected exception is `TableNotFoundException`: cluster works
as expected, the problem is in stale `tableId` passed from client, we clearly
understand why `tableId` might be stale.
In the second group come all the other exceptions. We can safely ignore any
exception from the first group, and it's better to log exceptions from second
one as we don't want to miss possible bugs.
The problem here is that the list of expected exceptions is not clearly
defined. More over, it may change between releases, so unexpected becomes
expected and vice versa.
Any undesired change in clients' behavior after cluster upgrade may be seen
as breaking change. Possible options are: leave as is, and bloat logs and disk
with tons of stack traces; turn off partition awareness and get performance
degradation; configure logger to ignore particular log event. I think, the last
option will be more popular, as it doesn't affect performance.
Does said above make sense to you?
--
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]
