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

updated dependency on sketches-core #3443

Merged
merged 5 commits into from
Sep 9, 2016
Merged

updated dependency on sketches-core #3443

merged 5 commits into from
Sep 9, 2016

Conversation

AlexanderSaydakov
Copy link
Contributor

Let's use the latest release of sketches-core. The only change required in the Druid code was to implement one more update method, which takes char[]. Historical data must be fully compatible.

@gianm
Copy link
Contributor

gianm commented Sep 8, 2016

was there any change in storage format?

if not then 👍

@AlexanderSaydakov
Copy link
Contributor Author

The serialized format must be the same.

@drcrallen drcrallen added this to the 0.9.2 milestone Sep 8, 2016
@AlexanderSaydakov
Copy link
Contributor Author

I didn't realize that Druid gets built for both JDK7 and JDK8. Support for JDK7 has been dropped in sketches-core-0.5.0. So if this is a barrier, the latest version we can use is 0.4.1

@AlexanderSaydakov
Copy link
Contributor Author

I was going to work on a new aggregator using latest sketches. This issue makes this plan questionable. Is there a plan to stop building Druid for JDK7?

@fjy
Copy link
Contributor

fjy commented Sep 8, 2016

@gianm
Copy link
Contributor

gianm commented Sep 9, 2016

@AlexanderSaydakov the approach taken with the caffeine (#3028), which also requires Java 8, is to have that particular extension target Java 8 but the rest of Druid target Java 7.

In this case since the datasketches extension already supported Java 7, we probably want to keep supporting it, but it should be ok to make a new datasketches-xxx extension for your new "xxx" aggregator.

@gianm
Copy link
Contributor

gianm commented Sep 9, 2016

👍 on the update to 0.4.1

@fjy
Copy link
Contributor

fjy commented Sep 9, 2016

👍

@fjy fjy merged commit 1a5042c into apache:master Sep 9, 2016
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.

4 participants