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

Refactoring default and best_compression codec to their algorithm name #8745

Conversation

sarthakaggarwal97
Copy link
Contributor

@sarthakaggarwal97 sarthakaggarwal97 commented Jul 18, 2023

Description

The aim of this PR is to refactor the default and best_compression codecs to their respective algorithm names.

Related Issues

Resolves #8695

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@sarthakaggarwal97 sarthakaggarwal97 changed the title Refactor codec Refactoring default and best_compression codec to their algorithm name Jul 18, 2023
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.remotestore.multipart.RemoteStoreMultipartIT.testStaleCommitDeletionWithInvokeFlush
      1 org.opensearch.remotestore.SegmentReplicationUsingRemoteStoreIT.testDropPrimaryDuringReplication
      1 org.opensearch.remotestore.RemoteStoreIT.testStaleCommitDeletionWithInvokeFlush
      1 org.opensearch.cluster.allocation.ClusterRerouteIT.testDelayWithALargeAmountOfShards

@codecov
Copy link

codecov bot commented Jul 18, 2023

Codecov Report

Merging #8745 (98377e7) into main (723cab6) will decrease coverage by 0.04%.
The diff coverage is 76.00%.

@@             Coverage Diff              @@
##               main    #8745      +/-   ##
============================================
- Coverage     71.16%   71.13%   -0.04%     
- Complexity    57274    57275       +1     
============================================
  Files          4759     4759              
  Lines        269881   269904      +23     
  Branches      39488    39492       +4     
============================================
- Hits         192067   191998      -69     
- Misses        61616    61740     +124     
+ Partials      16198    16166      -32     
Files Changed Coverage Δ
.../index/codec/customcodecs/Lucene95CustomCodec.java 76.92% <ø> (ø)
...java/org/opensearch/index/engine/EngineConfig.java 94.73% <68.42%> (-3.79%) ⬇️
.../java/org/opensearch/index/codec/CodecService.java 100.00% <100.00%> (ø)
...opensearch/index/codec/customcodecs/ZstdCodec.java 100.00% <100.00%> (ø)
...arch/index/codec/customcodecs/ZstdNoDictCodec.java 100.00% <100.00%> (ø)

... and 454 files with indirect coverage changes

@sarthakaggarwal97
Copy link
Contributor Author

We would need to create issues for documentation and OSB changes

@mgodwan
Copy link
Member

mgodwan commented Jul 18, 2023

@sarthakaggarwal97 With this change, how will the upgrades work for clusters with existing indices? Won't we need to support the existing enum values?

@sarthakaggarwal97
Copy link
Contributor Author

@sarthakaggarwal97 With this change, how will the upgrades work for clusters with existing indices? Won't we need to support the existing enum values?

@mgodwan yes thanks for pointing out, it would be breaking for the cluster upgrade. Will re-add the support for the old enums.

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@Poojita-Raj
Copy link
Contributor

@sarthakaggarwal97 With this change, how will the upgrades work for clusters with existing indices? Won't we need to support the existing enum values?

Agree with @mgodwan, we still need the support for "best_compression" and "default". A good follow up to this PR would be to make these changes in the documentation.

  1. https://opensearch.org/docs/1.0/opensearch/rest-api/create-index/ - In the documentation, we can be transparent about the actual codec name of the codecs being used for best_compression and default.
  2. Additionally, like we specify in the IllegalArgumentException for the index.codec setting, we should specify in the docs that we can provide one of these values: [default, best_compression, lz4, zlib, zstd, zstd_no_dict]

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.action.admin.cluster.node.tasks.ResourceAwareTasksTests.testTaskResourceTrackingDuringTaskCancellation

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.snapshots.DedicatedClusterSnapshotRestoreIT.testIndexDeletionDuringSnapshotCreationInQueue
      1 org.opensearch.cluster.allocation.AwarenessAllocationIT.testThreeZoneOneReplicaWithForceZoneValueAndLoadAwareness

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.

Refactor Index Codecs to expose algorithm names as index codec types
3 participants