Skip to content

Drop the deprecated NoneShardSpec#9246

Closed
jihoonson wants to merge 2 commits intoapache:masterfrom
jihoonson:remove-none-shardspec
Closed

Drop the deprecated NoneShardSpec#9246
jihoonson wants to merge 2 commits intoapache:masterfrom
jihoonson:remove-none-shardspec

Conversation

@jihoonson
Copy link
Contributor

A part of #9241. NoneShardSpec has been deprecated for a while.


This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added or updated version, license, or notice information in licenses.yaml
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths.
  • added integration tests.
  • been tested in a test Druid cluster.

@drcrallen
Copy link
Contributor

This breaks compatibility with segments that were built with none shard spec. My understanding is that very old segments are still to be supported to load on historicals for query.

@jihoonson
Copy link
Contributor Author

jihoonson commented Jan 23, 2020

Yes, this is an incompatible change. The NoneShardSpec was deprecated in March last year (#6883). I don't think we should support everything forever. For example, we don't support the old segment format (lower than the version 8) anymore.
However, I'm open to keep it for a couple of more releases. Do you think the next release is not a good time to remove it?

@gianm
Copy link
Contributor

gianm commented Jan 24, 2020

I don't think we should ever remove support for loading any previously-existing segments, no matter how old they are. To my knowledge, we haven't done this since Druid was open-sourced (if I recall correctly, the pre-v8 segment versions were also pre-OSS). This is because it is a pretty big burden on users to ask them to reindex all their data.

Does the fact that NoneShardSpec exists block #9241 from working?

If so, an alternative would be to remove NoneShardSpec, but include code to automatically convert metadata into something equivalent, so users don't need to reindex.

@jihoonson
Copy link
Contributor Author

Ok. This is not a blocker. I'm closing this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants