ctubbsii commented on a change in pull request #2101:
URL: https://github.com/apache/accumulo/pull/2101#discussion_r631468449
##########
File path:
server/base/src/main/java/org/apache/accumulo/server/ServiceEnvironmentImpl.java
##########
@@ -32,10 +34,12 @@
private final ServerContext srvCtx;
private final Configuration conf;
+ private final Map<TableId,Configuration> tableConfigs;
public ServiceEnvironmentImpl(ServerContext ctx) {
this.srvCtx = ctx;
this.conf = new ConfigurationImpl(srvCtx.getConfiguration());
+ this.tableConfigs = new ConcurrentHashMap<>();
Review comment:
This change only saves one line :smiley_cat:, but why not:
```suggestion
private final Map<TableId,Configuration> tableConfigs = new
ConcurrentHashMap<>();
public ServiceEnvironmentImpl(ServerContext ctx) {
this.srvCtx = ctx;
this.conf = new ConfigurationImpl(srvCtx.getConfiguration());
```
##########
File path:
server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java
##########
@@ -783,6 +788,11 @@ public void executeReadAhead(KeyExtent tablet,
ScanDispatcher dispatcher, ScanSe
scanExecutors.get("meta").execute(task);
} else {
DispatchParameters params = new DispatchParamsImpl() {
+
+ // in scan critical path so only create ServiceEnv if needed
+ private final Supplier<ServiceEnvironment> senvSupplier =
+ Suppliers.memoize(() -> new ServiceEnvironmentImpl(context));
Review comment:
Okay, so I think I understand: it depends on the dispatcher
implementation, and some may not need to reference the environment in order to
dispatch. So, we just avoid instantiating it unless they actually use it, just
to avoid a performance hit, because the dispatch method gets called a lot. Is
that right?
Aside:
I noticed that the service environment passed in init could contain
different information than the one passed in dispatch. That could be confusing
for plugin implementers who might expect them to agree. Just a quirk with the
current design of the service environment containing snapshots and then caching
them.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
