zachjsh commented on code in PR #13165:
URL: https://github.com/apache/druid/pull/13165#discussion_r1014383800
##########
extensions-core/druid-catalog/src/main/java/org/apache/druid/catalog/sync/CachedMetadataCatalog.java:
##########
@@ -84,14 +97,14 @@ protected TableEntry(TableMetadata table)
{
private final SchemaSpec schema;
private long version = NOT_FETCHED;
- private final ConcurrentHashMap<String, TableEntry> cache = new
ConcurrentHashMap<>();
+ private final Map<String, TableEntry> cache = new TreeMap<>();
Review Comment:
Seems like using treeMap here, and adding sychronization at method level
really lowers the granularity of concurrent access; now we have entire
functions being synchronized, thought better to use concurrentMap, so that
updates to each table are synchronized, but not each table can be updated
independently and concurrently with another. I notice that you had the `update`
method in this class as synchronized before even using treeMap, why is this?
Sorry for the confusion on the comment I made, I was only referring to that
local List variable, not the entire cache.
--
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]
