You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2022/12/30 21:48:52 UTC
[GitHub] [accumulo-proxy] DomGarguilo opened a new pull request, #60: WIP - Update compaction techniques
DomGarguilo opened a new pull request, #60:
URL: https://github.com/apache/accumulo-proxy/pull/60
This PR aims to replace the deprecated compaction methods with the new ones.
Fixes #47
* a new thrift object was added for the PluginConfig
* The compaction test was replaced with a test for supplying a custom `CompactionSelector`
I'm not sure that this is the best place to have the `CompactionSelector` as it is only used for testing so I will probably move it but just wanted to get something working together first.
I also plan on looking into adding a test for custom `CompactionConfigurer` too.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo-proxy] DomGarguilo commented on pull request #60: Update compaction techniques
Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on PR #60:
URL: https://github.com/apache/accumulo-proxy/pull/60#issuecomment-1385972455
Everything looks good here to me and the tests pass when run with my IDE but when running in the terminal, there is an error `Security sealing violation: package org.apache.accumulo.proxy is sealed`. Not sure exactly whats going on here but I'm looking at moving the `CompactionSelector` that is currently in the test directory into its own Jar file.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo-proxy] ctubbsii commented on pull request #60: Update compaction techniques
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on PR #60:
URL: https://github.com/apache/accumulo-proxy/pull/60#issuecomment-1374019983
I don't know enough about the new compaction configuration to review this. I requested a review from @keith-turner or @dlmarion , who I think could assess it better.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo-proxy] dlmarion commented on a diff in pull request #60: Update compaction techniques
Posted by GitBox <gi...@apache.org>.
dlmarion commented on code in PR #60:
URL: https://github.com/apache/accumulo-proxy/pull/60#discussion_r1072358091
##########
src/main/java/org/apache/accumulo/proxy/ProxyServer.java:
##########
@@ -403,12 +404,24 @@ public void compactTable(ByteBuffer login, String tableName, ByteBuffer startRow
.setStartRow(ByteBufferUtil.toText(startRow)).setEndRow(ByteBufferUtil.toText(endRow))
.setIterators(getIteratorSettings(iterators)).setFlush(flush).setWait(wait);
- if (compactionStrategy != null) {
- org.apache.accumulo.core.client.admin.CompactionStrategyConfig ccc = new org.apache.accumulo.core.client.admin.CompactionStrategyConfig(
- compactionStrategy.getClassName());
- if (compactionStrategy.options != null)
- ccc.setOptions(compactionStrategy.options);
- compactionConfig.setCompactionStrategy(ccc);
+ if (selectorConfig != null) {
+ Map<String,String> options = selectorConfig.options == null ? Map.of()
+ : selectorConfig.options;
+
+ org.apache.accumulo.core.client.admin.PluginConfig spc = new org.apache.accumulo.core.client.admin.PluginConfig(
+ selectorConfig.getClassName(), options);
+
+ compactionConfig.setSelector(spc);
+ }
+
+ if (configurerConfig != null) {
+ Map<String,String> options = configurerConfig.options == null ? Map.of()
+ : configurerConfig.options;
+
+ org.apache.accumulo.core.client.admin.PluginConfig cpc = new org.apache.accumulo.core.client.admin.PluginConfig(
+ configurerConfig.getClassName(), options);
+
+ compactionConfig.setConfigurer(cpc);
Review Comment:
This new logic looks correct, appears to match the code in `CompactCommand.setupConfigurableCompaction`.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo-proxy] ctubbsii merged pull request #60: Update compaction techniques
Posted by GitBox <gi...@apache.org>.
ctubbsii merged PR #60:
URL: https://github.com/apache/accumulo-proxy/pull/60
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org