-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
remove json column v4 format writers #17625
Conversation
new NestedColumnV4HandlerProvider() | ||
); | ||
} else { | ||
if (formatsConfig.getNestedColumnFormatVersion() == null || formatsConfig.getNestedColumnFormatVersion() == 5) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was hoping there would be a place where we fail the query/service if nestedColVersion is set to 4. Could not find such a guardrail. Is that not required ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A failure would happen in DefaultColumnFormatConfig
which throws an exception if the format is not 5.
I guess I could think about switching this over to use the PropertiesValidator
introduced in #17634, though this config itself isn't really deprecated, I did intend that thing to be used for validation of runtime.properties.
The more I think about it, I'm not actually sure failing is that useful, since v5 isn't really much of a behavior change, the risk is that with v5 segments you cannot roll back to Druid versions older than 26.0.0, which was covered in prior release notes and doesn't seem to be worth exploding over. The main difference of v5 format is that there are nested array fields which means you can use json_value function to get arrays of primitives out more efficiently and there are optimizations for the cases where there are no actual objects in a json column (e.g. it writes a scalar column instead if there is no nesting), so there isn't really much of a behavioral risk to using the new format.
I think we could also consider just log.warn about having a bad config value that is no longer supported and just ignoring it, though I suppose older druid versions also fail if you didn't put the value of 4 or 5 into the config, so 🤷
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, we shouldn't fail if a user provides formatVersion: 4
, b/c the format version is provided in ingestion specs and this will cause previously-working ingestion specs to stop working after an upgrade. It's OK if a validation prevents the server from starting up completely, but it's IMO less OK if a validation allows the server to start up but then breaks certain operations that used to work. Better to log a warning, IMO.
CI failures are complaining about the log statements not being covered, which i think is ok to ignore |
Tagged "release notes" because there are implications for upgrading from certain older versions. (Which version was v5 made the default in, btw?) |
V5 was made default in 28.0, and was introduced in 26.0, updated the release note entry in PR description to include this |
Description
Removes the ability to write JSON columns in the older version '4' format, leaving only the latest current version, '5', as the only option. (Internally it isn't really v5, rather v0 of the 'auto' format, but it is branded as v5 for the sake of continuity).
I have left the structures to control which version is written added in #14955, in the event we need to introduce any newer versions in the future, we just only allow v5 right now.
Release note
JSON columns can no longer be written in the older V4 format. The current latest version, V5, which has been the default since 28.0.0 is now the only version which can be written to new segments. This means both the
formatVersion
of a JSON column spec on JSON based ingestion specs, e.g.and
druid.indexing.formats.nestedColumnFormatVersion
in runtime.properties only allow 5.All older version of JSON columns can still be read and queried just fine, this change just prevents writing new segments in the older format.
V5 was introduced in Druid 26.0.0, so anyone upgrading from Druid 25.0.0 or older and using JSON columns may wish to first upgrade to any version between 26.0.0 and 32.0.0, which can be configured to keep writing V4 columns until operators are sure there is no need to roll back an upgrade to the older version.