Skip to content

Conversation

@DomGarguilo
Copy link
Member

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.

@DomGarguilo DomGarguilo changed the title WIP - Update compaction techniques Update compaction techniques Jan 4, 2023
@DomGarguilo DomGarguilo requested a review from ctubbsii January 4, 2023 16:12
@ctubbsii
Copy link
Member

ctubbsii commented Jan 6, 2023

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.

org.apache.accumulo.core.client.admin.PluginConfig cpc = new org.apache.accumulo.core.client.admin.PluginConfig(
configurerConfig.getClassName(), options);

compactionConfig.setConfigurer(cpc);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new logic looks correct, appears to match the code in CompactCommand.setupConfigurableCompaction.

# Conflicts:
#	src/test/java/org/apache/accumulo/proxy/its/SimpleProxyBase.java
@DomGarguilo
Copy link
Member Author

DomGarguilo commented Jan 17, 2023

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 that gets thrown when running the compaction IT that was modified in this PR. 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.

…elector.java to src/test/java/org/apache/accumulo/proxy/its/SelectHalfSelector.java

Fix jar sealing error
Fix jar sealing import
Copy link
Member

@ctubbsii ctubbsii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed the merge conflicts and the jar sealing issue. The changes look okay to me, and it was previously approved, so I'll merge it. The ITs are still failing, but that's for another issue.

@ctubbsii ctubbsii merged commit 52bb2bc into apache:main Jan 18, 2023
@DomGarguilo DomGarguilo deleted the compactionUpdate branch January 18, 2023 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update compaction techniques

3 participants