Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Segment Replication] Support for custom codecs #7668

Closed
dreamer-89 opened this issue May 22, 2023 · 4 comments
Closed

[Segment Replication] Support for custom codecs #7668

dreamer-89 opened this issue May 22, 2023 · 4 comments
Assignees
Labels
enhancement Enhancement or improvement to existing feature or request Indexing:Replication Issues and PRs related to core replication framework eg segrep

Comments

@dreamer-89
Copy link
Member

Is your feature request related to a problem? Please describe.
Support rolling upgrades for plugins/integrations providing custom codec implementation e.g. k-NN. This will be fixed in core via #7349, this issue is about fixing the bridge

Parent: #3881
Fix on core: #7349

Describe the solution you'd like
On core rolling upgrades are fixed by #7349. Steps followed in this fix:

In order to support custom codec, primary shard should read the custom OS version to codec mapping and then load the appropriate implementation (already available in classpath for kNN plugin).

@Poojita-Raj
Copy link
Contributor

List of plugins that do not need any further changes (do not extend codec service):

  • opensearch-alerting
  • opensearch-anomaly-detection
  • opensearch-asynchronous-search
  • opensearch-cross-cluster-replication
  • opensearch-geospatial
  • opensearch-index-management
  • opensearch-job-scheduler
  • opensearch-ml
  • opensearch-neural-search
  • opensearch-notifications
  • opensearch-notifications-core
  • opensearch-observability
  • opensearch-performance-analyzer
  • opensearch-reports-scheduler
  • opensearch-security
  • opensearch-sql

Plugins that require further investigation to verify if they support rolling upgrades for their custom codecs:

  • opensearch-knn
  • opensearch-security-analytics

@Poojita-Raj
Copy link
Contributor

Poojita-Raj commented Jun 7, 2023

To verify the working of custom codecs when we include the fix from #7698

Remake distribution on 2.8 branch + use 2.8 release knn -> upgrade to 2.x (2.9) with changes from #7698 + use 2.9 snapshot of knn

  • try regular indexing with knn.index = true
  • try knn mapping indexing with knn.index = true

Remake distribution on 2.8 branch + use 2.8 release knn + lower codec to lucene94 + update mapping of version to codec -> upgrade to 2.x (2.9) with changes from #7698 + use 2.9 snapshot of knn + update mapping of version to codec

  • try regular indexing with knn.index = true
  • try knn mapping indexing with knn.index = true

This is to verify whether further changes are required to enable rolling upgrades for clusters with segment replication and knn enabled.

@Poojita-Raj
Copy link
Contributor

While upgrading from modified 2.8 distribution that's using lucene94 codec, we see that in the segment replications taking place, we are still loading in KNN950Codec instead of the KNN940Codec that is expected on using Lucene94.

In KNNCodecService.java:

    @Override
    public Codec codec(String name) {
        return KNNCodecVersion.current().getKnnCodecSupplier().apply(super.codec(name), mapperService);
    }

In the above code - KNNCodecVersion.current() points to V_9_5_0 which uses KNN950Codec as a wrapper.
So non-KNN related operations would use Lucene94, but KNN related operations would still use Lucene95.

In a true codec bump, we would be sending over higher version KNNCodec during segment replication which wouldn't be able to be read by lower codec version nodes in a rolling upgrade.

In order to support rolling upgrades for segment replication and knn enabled:

  • Modify overridden method codec() in KNNCodecService.java to load the appropriate KNN Codec based on codec passed into method.

For added security, should also only allow this operation if rolling upgrade is in progress.

@Bukhtawar Bukhtawar added the Indexing:Replication Issues and PRs related to core replication framework eg segrep label Jul 27, 2023
@Poojita-Raj
Copy link
Contributor

Closing this out as this implementation of the solution was discarded in favor of introducing the shard_movement_strategy setting as mentioned in #3881

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement or improvement to existing feature or request Indexing:Replication Issues and PRs related to core replication framework eg segrep
Projects
Status: Done
Development

No branches or pull requests

4 participants