Skip to content

Use compaction dynamic config to enable compaction supervisors#17782

Merged
kfaraz merged 8 commits intoapache:masterfrom
kfaraz:use_compact_dynamic_config_to_enable_supervisors
Mar 23, 2025
Merged

Use compaction dynamic config to enable compaction supervisors#17782
kfaraz merged 8 commits intoapache:masterfrom
kfaraz:use_compact_dynamic_config_to_enable_supervisors

Conversation

@kfaraz
Copy link
Copy Markdown
Contributor

@kfaraz kfaraz commented Mar 7, 2025

Description

#16768 introduces the option to run compaction as a supervisor on Overlord instead of via coordinator duties.
It also packages several improvements over the traditional compaction duty, such as support for using
the MSQ engine to launch compaction tasks.

However, that patch relies on an Overlord runtime property to enable/disable this feature, which can be
cumbersome and error prone, given that both Coordinator and Overlord need to be aware of it.
It also leads to a poorer design since half of compaction-related features are controlled by the compaction
dynamic config whereas the other half is controlled by the runtime properties.

This patch reconciles the two by simply using compaction dynamic config for all purposes.
It has the added benefit of allowing a quick switch between Overlord supervisor and Coordinator duty,
for ease of testing and in case of unexpected behaviour in production.

Changes

  • Remove runtime property object CompactionSupervisorConfig
  • Add fields useSupervisors and engine to cluster-level compaction dynamic config
  • Remove unused field useAutoScaleSlots

Release note

Remove following Overlord runtime properties:

  • druid.supervisors.compaction.enabled
  • druid.supervisors.compaction.engine

Add new fields to coordinator compaction config:

  • useSupervisors - Enable compaction to run as a supervisor on the Overlord instead of as a coordinator duty
  • engine - Choose between native and msq engine to run compaction tasks. msq can be used only when useSupervisors is true.

Remove coordinator dynamic config useAutoScaleSlots as it is not used anymore.


This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • a release note entry in the PR description.
  • 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, ensuring the threshold for code coverage is met.
  • added integration tests.
  • been tested in a test Druid cluster.

= verifyAndGetPayload(resource.getCompactionConfig(), DruidCompactionConfig.class);

Response response = resource.setCompactionTaskLimit(0.5, 9, true, mockHttpServletRequest);
Response response = resource.setCompactionTaskLimit(0.5, 9, mockHttpServletRequest);

Check notice

Code scanning / CodeQL

Deprecated method or constructor invocation Note test

Invoking
CoordinatorCompactionConfigsResource.setCompactionTaskLimit
should be avoided because it has been deprecated.
@vogievetsky vogievetsky added the Needs web console change Backend API changes that would benefit from frontend support in the web console label Mar 12, 2025
@vogievetsky
Copy link
Copy Markdown
Contributor

Do the values of useSupervisors and engine have an effect on the applicability of compactionTaskSlotRatio and maxCompactionTaskSlots ? Specifically if useSupervisors: true and engine: "msq" do compactionTaskSlotRatio and maxCompactionTaskSlots get ignored or are they equally applicable?

|`maxCompactionTaskSlots`|Maximum number of task slots that can be taken up by compaction tasks.|2147483647 (i.e. total task slots)|
|`compactionPolicy`|Policy to prioritize intervals for compaction|`newestSegmentFirst`|
|`useSupervisors`|Whether compaction should be run on Overlord using supervisors instead of Coordinator duties.|false|
|`engine`|Engine to use for running compaction tasks, native or MSQ.|Native|
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should list the values exactly as you feed them into the API like "native" or "msq". Does this property only apply if useSupervisors: true? if so it should be noted.

|------|-----------|-------------|
|`compactionTaskSlotRatio`|Ratio of number of slots taken up by compaction tasks to the number of total task slots across all workers.|0.1|
|`maxCompactionTaskSlots`|Maximum number of task slots that can be taken up by compaction tasks.|2147483647 (i.e. total task slots)|
|`compactionPolicy`|Policy to prioritize intervals for compaction|`newestSegmentFirst`|
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the description list all the possible values for this property please?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I noticed that the return of GET /druid/coordinator/v1/config/compaction has a slightly different shape.

image

What is priorityDatasource? Can the same shape of object be submitted?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried posting

image

and got "Could not resolve subtype of [simple type, class org.apache.druid.server.compaction.CompactionCandidateSearchPolicy]: missing type id property 'type' (for POJO property 'compactionPolicy')" so maybe it HAS to be that shape.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, should have kept this PR in draft. Still working on the docs.
But yes, the result of the GET is the correct format.

"maxCompactionTaskSlots": 1500,
"compactionPolicy": "newestSegmentFirst",
"useSupervisors": true,
"engine": "MSQ"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be "msq" (lowercase)?

@vogievetsky
Copy link
Copy Markdown
Contributor

Is the /druid/coordinator/v1/config/compaction/cluster API added in this PR?

@kfaraz
Copy link
Copy Markdown
Contributor Author

kfaraz commented Mar 14, 2025

Is the /druid/coordinator/v1/config/compaction/cluster API added in this PR?

No, @vogievetsky , this API was added a while back but some new fields are being added to this API in this PR.

Thanks for the docs suggestions, I will update them accordingly.

@kfaraz kfaraz marked this pull request as draft March 14, 2025 16:33
@kfaraz kfaraz marked this pull request as ready for review March 15, 2025 23:07
@kfaraz kfaraz requested a review from vogievetsky March 16, 2025 11:28
Comment thread docs/data-management/automatic-compaction.md
|`compactionPolicy.type`|Policy to choose intervals for compaction. Currently, the only supported policy in `newestSegmentFirst`.|`newestSegmentFirst`|
|`compactionPolicy.priorityDatasource`|Datasource to prioritize for compaction. The intervals of this datasource are chosen for compaction before the intervals of any other datasource. Within this datasource, the intervals are prioritized based on the chosen compaction policy.|None|
|`useSupervisors`|Whether compaction should be run on Overlord using supervisors instead of Coordinator duties.|false|
|`engine`|Engine used for running compaction tasks. Possible values are `native` and `msq`. `msq` engine can be used for compaction only if `useSupervisors` is `true`.|`native`|
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe should this clarify that this is the default if not specified in the datasource specific config?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, fair point. Let me update that.

Comment on lines +270 to +271
|`compactionPolicy.type`|Policy to choose intervals for compaction. Currently, the only supported policy in `newestSegmentFirst`.|`newestSegmentFirst`|
|`compactionPolicy.priorityDatasource`|Datasource to prioritize for compaction. The intervals of this datasource are chosen for compaction before the intervals of any other datasource. Within this datasource, the intervals are prioritized based on the chosen compaction policy.|None|
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think maybe this should be split out and documented as a CompactionPolicy since it is specific to newestSegmentFirst in case we ever add other policies (or make this one more flexible or whatever)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that makes sense.

@vogievetsky
Copy link
Copy Markdown
Contributor

I tried turning off supervisor based auto-compaction by submitting:

image

to /druid/coordinator/v1/config/compaction/cluster and got

{
    "error": "druidException",
    "errorCode": "invalidInput",
    "persona": "USER",
    "category": "INVALID_INPUT",
    "errorMessage": "MSQ Compaction engine can be used only with compaction supervisors.",
    "context": {}
}

since I asked for useSupervisors to be false I feel like it should be ok to omit the engine (since it defaults to native anyway)

I would expect this API call to work to be consistent with the rest of Druid.

@kfaraz
Copy link
Copy Markdown
Contributor Author

kfaraz commented Mar 22, 2025

@vogievetsky , had you updated the config in a prior API call to use engine: msq?
You can verify that by doing a GET /druid/coordinator/v1/config/compaction.
(On that note, I think we should also add a separate GET /druid/coordinator/v1/config/compaction/cluster).

The update API POST /druid/coordinator/v1/config/compaction/cluster only updates the configs
that have been specified in the payload and leaves the rest unchanged.
This was done to align with the API POST /druid/coordinator/v1/config (i.e. update coordinator dynamic config)
which behaves in a similar way.

I would personally prefer if both of these APIs (or atleast the new one) just updated the whole object
(since that is what most users would expect anyway).

Please let me know what you think. I will make the necessary changes.

@vogievetsky
Copy link
Copy Markdown
Contributor

Yes, I have previously set engine to msq. I guess I always imagined all these APIs to take a entire object, validate it, and then set it. That was my mental model for the /druid/coordinator/v1/config also and I guess now I need to go and review some code. I would strongly prefer if the new API could take the entire payload as the config and not merge it with the existing payload. We can figure out what to do with older APIs that do that merge later.

@kfaraz
Copy link
Copy Markdown
Contributor Author

kfaraz commented Mar 22, 2025

Sounds good. Thanks for the clarification, @vogievetsky .
Do you think it would also help to have a separate GET /druid/coordinator/v1/config/compaction/cluster
to fetch just the cluster config and not the whole compaction config (including all datasources)?

@kfaraz
Copy link
Copy Markdown
Contributor Author

kfaraz commented Mar 23, 2025

@vogievetsky , I am merging this PR for now.
Will create a follow up PR for any further changes.

@kfaraz kfaraz merged commit 5fa69bd into apache:master Mar 23, 2025
75 checks passed
@kfaraz kfaraz deleted the use_compact_dynamic_config_to_enable_supervisors branch March 23, 2025 02:59
@vogievetsky
Copy link
Copy Markdown
Contributor

Thank you for addressing my feedback. I think a separate API to fetch just the cluster config would be really great. The current API is very messy.

cecemei pushed a commit to cecemei/druid that referenced this pull request Mar 31, 2025
…e#17782)

Changes
---------
- Remove runtime property object `CompactionSupervisorConfig`
- Add fields `useSupervisors` and `engine` to cluster-level compaction dynamic config
- Remove unused field `useAutoScaleSlots`
kfaraz added a commit that referenced this pull request Apr 2, 2025
)

Changes
---------
- Add new Overlord APIs to read and update compaction status and configs as follows:
  - If `useSupervisors` is true, read and write compaction supervisors
using the `CompactionScheduler`or the `SupervisorManager`
  - If `useSupervisors` is false, read and write compaction configs using `CoordinatorConfigManager`
- Move some logic from `CoordinatorCompactionConfigsResource` to `CoordinatorConfigManager`
- Use `CoordinatorConfigManager` in `OverlordCompactionResource` to manipulate compaction configs
- Add `CompactionSupervisorManager` to act as a wrapper over `SupervisorManager` and allow
manipulation of compaction supervisor specs
- Fix initialization bug in `OverlordCompactionScheduler` introduced in #17782
@kgyrtkirk kgyrtkirk added this to the 33.0.0 milestone Apr 14, 2025
gianm added a commit that referenced this pull request Apr 25, 2025
)

* Some debug configs

* use postgresql as the default metadata store and set a few debug log

* Add s3 extension, update local storage directory, use emoji in website title

* Update favicon, easier to find the console tab

* Add indexer server, add some basic security config, updated historical and broker to use the common druid root directory

* Some policy config

* add checks for SegmentMetadataQuery

* Add thread.sleep for flaky.

* auth config

* format, and remove temp folder rules

* added NoopPolicyEnforcer and RestrictAllTablesPolicyEnforcer class

* Support pushing and streaming task payload for HDFS (#17742)

Implement pushTaskPayload/streamTaskPayload as introduced in #14887
for HDFS storage to allow larger mm-less ingestion payloads when using
HDFS as the deep storage location.

* Remove usages of deprecated API Files.write() (#17761)

* Add deprecated com.google.common.io.Files#write to forbiddenApis

* Replace deprecated Files.write()

* Doc: Fix description typo for sqlserver metadata store (#17771)

Mistakenly categories under deep storage instead of metadata store.

* Fix binding of segment metadata cache on CliOverlord (#17772)

Changes
---------
- Bind `SegmentMetadataCache` only once to
`HeapMemorySegmentMetadataCache` in `SQLMetadataStorageDruidModule`
- Invoke start and stop of the cache from `DruidOverlord` rather than on lifecycle start/stop
- Do not override the binding in `CliOverlord`

* Docs: Remove semicolon from example (#17759)

* Restrict segment metadata kill query till maxInterval from last kill task time (#17770)

Changes
---------
- Use `maxIntervalToKill` to determine search interval for killing unused segments.
- If no segment has been killed for the datasource yet, use durationToRetain

* Update the Supervisor endpoint to not restart the Supervisor if the spec was unmodified (#17707)

Add an optional query parameter called skipRestartIfUnmodified to the
/druid/indexer/v1/supervisor endpoint. Callers can set skipRestartIfUnmodified=true
to not restart the supervisor if the spec is unchanged.

Example:

curl -X POST --header "Content-Type: application/json" -d @supervisor.json
localhost:8888/druid/indexer/v1/supervisor?skipRestartIfUnmodified=true

* Reduce noisy coordinator logs (#17779)

* Emit time lag from Kafka supervisor (#17735)

Changes
---------
- Emit time lag from Kafka similar to Kinesis as metrics `ingest/kafka/lag/time`,
`ingest/kafka/maxLag/time`, `ingest/kafka/avgLag/time`
- Add new method in `KafkaSupervisor` to fetch timestamps of latest records in stream to compute time lag
- Add new field `emitTimeLagMetrics` in `KafkaSupervisorIOConfig` to toggle emission of new metrics

* fix processed row formatting (#17756)

* Web console: add suggestions for table status filtering. (#17765)

* suggest filter values when known

* update snapshots

* add more d

* fix load rule clamp

* better segment timeline init

* Remove all usages of skife config (#17776)


Changes
---------
- Usages of skife config had been deprecated in #14695 and
`LegacyBrokerParallelMergeConfig` is the last config class that still uses it.
- Remove `org.skife.config` from pom, licenses, log4j2.xml, etc.
- Add validation for deleted property paths in `StartupInjectorBuilder.PropertiesValidator`
- Use the replacement flattened configs (which remove the `.task` and `.pool` substring)

* Add field `taskLimits` to worker select strategies (#16889)

Changes
---------
- Add field `taskLimits` to the following worker select strategies
`equalDistribution`, `equalDistributionWithCategorySpec`, `fillCapacityWithCategorySpec`, `fillCapacity`
- Add sub-fields `maxSlotCountByType` and `maxSlotRatioByType` to `taskLimits`
- Apply these limits per worker when assigning new tasks

---------
Co-authored-by: sviatahorau <mikhail.sviatahorau@deep.bi>
Co-authored-by: Benedict Jin <asdf2014@apache.org>
Co-authored-by: Kashif Faraz <kashif.faraz@gmail.com>

* remove NullValueHandlingConfig, NullHandlingModule, NullHandling (#17778)

* Docs: Add SQL query example (#17593)

* Docs: Add query example

* Update after review

* Update query

* Update docs/api-reference/sql-api.md

---------

Co-authored-by: Victoria Lim <vtlim@users.noreply.github.com>

* More logging cleanup on Overlord (#17780)

* Remove maven.twttr repo from pom (#17797)

remove usage of dependency:go-offline from build scripts - as it tries to download excluded artifacts

---------

Co-authored-by: Zoltan Haindrich <kirk@rxd.hu>

* fix bug (#17791)

* Log query stack traces for DEVELOPER and OPERATOR personas. (#17790)

Currently, query stack traces are logged only when "debug: true" is set
in the query context. This patch additionally logs stack traces targeted
at the DEVELOPER or OPERATOR personas, because for these personas, stack
traces are useful more often than not.

We continue to omit stack traces by default for USER and ADMIN, because
these personas are meant to interact with the API, not with code or logs.
Skipping stack traces minimizes clutter in the logs.

* Set useMaxMemoryEstimates=false for MSQ tasks (#17792)

* Web console: fix go to task selecting correct task type (#17788)

* fix go to task selecting correct task type

* support autocompact also

* support scheduled_batch, refactor

* one more state and update tests

* Enable ComponentSuppliers to run queries using Dart (#17787)



Enables Calcite*Test-s and quidem tests to run queries with Dart.

needed some minor tweaks:

    changed to use interfaces at some places
    renamed DartWorkerClient to DartWorkerClientImpl and made DartWorkerClient an interface
    reused existing parts of the MSQ test system to run the query

* Fix single container config creates failing peon tasks (#17794)

* Fix single container config creates failing peon tasks

* More obvious array error output

* Update `k8s-jobs.md` reference (#17805)

Signed-off-by: Emmanuel Ferdman <emmanuelferdman@gmail.com>

* Footer Copyright Year Update (#17751)

* Update docusaurus.config.js

* Update docusaurus.config.js

* [Revert] Reduce number of metadata transaction retries (#17808)

* Revert "Run JDK 21 workflows with latest JDK. (#17694)" (#17806)

* Revert "Run JDK 21 workflows with latest JDK. (#17694)"

This reverts commit 31ede5c

* Review comments.

* Review comments.

* Revert "reject publishing actions with a retriable error code if a earlier task is still publishing (#17509)"

This reverts commit aca56d6.

* Fix unstable tests after #17787 and dart usage in quidem-ut (#17814)

* fixes

* fix cleanup

* Use "mix" shuffle spec for target size with nil clusterBy. (#17810)

When a nil clusterBy is used, we have no way of achieving a particular
target size, so we need to fall back to a "mix" spec (unsorted single
partition).

This comes up for queries like "SELECT COUNT(*) FROM FOO LIMIT 1" when
results use a target size, such as when we are inserting into another
table or when we are writing to durable storage.

* Docs: Recommend using runtime property javaOptsArray instead of javaOpts

* Add minor checks in jetty utils (#17817)

Add minor checks in jetty utils class

* CI improvement: Leverage cancelled() instead of always() for CI jobs (#17819)

* Make MSQ tests use the same datasets as other similar tests (#17818)

MSQ tests had their own way of creating the segments/etc - this have lead to that custom datasets didn't worked with them.
This patch alters a few things to make it possible to access CompleteSegment for the active segments - which fixed the issue and also enabled the removal of the extra loading codes.

* Add unnest tests to quidem (#17825)

This PR adds the sql-native unnest tests to quidem. This set of tests has 6392 queries in total, with 5247 positive tests and 1145 negative tests.

* Web console: show loader on aux queries (#17804)

* show loader on aux queries

* show supervisors if not on page 0

* refactor

* fix bug fetching data when columns are added or removed

* update test

* Use compaction dynamic config to enable compaction supervisors (#17782)

Changes
---------
- Remove runtime property object `CompactionSupervisorConfig`
- Add fields `useSupervisors` and `engine` to cluster-level compaction dynamic config
- Remove unused field `useAutoScaleSlots`

* Retry segment publish task actions without holding locks (#17816)

#17802 reverted a retry of failed segment publish actions.

This patch attempts to address the original issue by retrying the segment publish task actions
on the client (i.e. task) side without holding any locks so that other transactions are not blocked.
Changes

    Add retries to TransactionalSegmentPublisher
    Add field retryable to SegmentPublishResult
    Remove class DataStoreMetadataUpdateResult and use SegmentPublishResult instead

* Add the capability to turboload segments onto historicals (#17775)

Add the capability to set Historicals into a turbo loading mode,
to focus on loading segments at the cost of query performance.

Context
--------
Currently, when a new Historical is started, it initially starts out using a bootstrap thread pool.
It uses this thread pool to load any existing cached segments and broadcast segments.
Once it loads any segments from both these sources, the historical switches to a smaller thread-pool
and begins to serve queries.

In certain cases, it would be useful to have the historical switch back to this mode,
and focus on loading segments, either to continue loading the initial non-bootstrap segments,
or to catch up with assigned segments.

This PR adds a coordinator dynamic config that allows servers to be configured to use
the larger bootstrap threadpool to load segments faster.

Changes
---------
- Added a new dynamic coordinator configuration, `turboLoadingNodes`.
- Ignore  `druid.coordinator.loadqueuepeon.http.batchSize` for servers in `turboLoadingNodes`
- Add API on historical to return loading capabilities i.e. num loading threads in normal and turbo mode

* Fix resource leak for GroupBy query merge buffer when query matched result cache (#17823)

* Fix resource leak for GroupBy query merge buffer when match result cache

* Fix resource leak for GroupBy query merge buffer when match result cache

* Add test

* Add test

* Add comment

* Add test

* Add metric and simulation test for turbo loading mode (#17830)

Changes
---------
- Add field `loadingMode` to `SegmentChangeStatus`
- Including loading mode in `DataSegmentChangeResponse`
- Include loading mode in the `description` of metrics emitted from `HttpLoadQueuePeon`
- Add simulation test to verify loading mode metrics

* Update query example (#17811)

* String util upgrade for jdk9+ (#17795)

* Update StringUtils.replace() after fix in JDK9

* Upgrade optimized string replace algorithm

* Update methods by re-using declared StringUtils#replace method

* Replace hard-coded UTF-8 encodings with StandardCharsets

* Documentation Fix (#17826)

* Enable to run quidem tests against multiple configurations; add conditionals; cleanup framework init (#17829)

* cleans up `SqlTestFramework` initialization to leave the `OverrideModule` empty - so that tests could more easily take over parts
* remove the `QueryComponentSupplier#createEngine`  factory method - instead uses a `Class<SqlEngine>` and use the `injector` to initialize it
* enables the usage of `!disabled <supplier> <message>` - to mark cases which are not yet supported with a specific configuration for some reason
* fixes that `datasets` was not respecting the `rollup` specification of the ingest
* enables to use `MultiComponentSupplier` backed tests - these will turn into matrix tests over multiple componentsuppliers - enabling running the same testcase in different scenarios

* Fix failing test in DimensionSchemaUtilsTest (#17832)

* Improve performance of segment metadata cache on Overlord (#17785)

Description
-----------
#17653 introduces a cache for segment metadata on the Overlord.
This patch is a follow up to that to make the cache more robust, performant and debug-friendly.

Changes
---------
- Do not cache unused segments
This significantly reduces sync time in cases where the cluster has a lot of unused segments.
Unused segments are needed only during segment allocation to ensure that a duplicate ID is not allocated.
This is a rare DB query which is supported by sufficient indexes and thus need not be cached at the moment.
- Update cache directly when segments are marked as unused to avoid race conditions with DB sync.
- Fix NPE when using segment metadata cache with concurrent locks.
- Atomically update segment IDs and pending segments in a `HeapMemoryDatasourceSegmentCache`
using methods `syncSegmentIds()` and `syncPendingSegments()` rather than updating one by one.
This ensures that the locks are held for a shorter period and the update made to the cache is atomic.

Main updated classes
----------------------
- `IndexerMetadataStorageCoordinator`
- `OverlordDataSourcesResource`
- `HeapMemorySegmentMetadataCache`
- `HeapMemoryDatasourceSegmentCache`

Cleaner cache sync
--------------------
In every sync, the following steps are performed for each datasource:

- Retrieve ALL used segment IDs from metadata store
- Atomically update segment IDs in cache and determine list of segment IDs which need to be refreshed.
- Fetch payloads of segments that need to be refreshed
- Atomically update fetched payloads into the cache
- Fetch ALL pending segments
- Atomically update pending segments into the cache
- Clean up empty intervals from datasource caches

* GroupBy: Fix offsets on outer queries. (#17837)

Prior to this patch, an offset specified on a groupBy that itself has an
inner groupBy would lead to an error like "Cannot push down offsets". This
happened because of a violated assumption: the processing logic assumes that
offsets have been pushed into limits (so limit pushdown optimizations can
safely be used).

This patch adjusts processing to incorporate offsets into limits during
processing of subqueries. Later on, in post-processing, offsets are applied
as written.

* Enable build cache for web-console (#17831)

* run audit fix (#17836)

* Do not block task actions on Overlord if segment metadata cache is syncing (#17824)

* Do not use segment metadata cache until leader has synced

* Read from cache only when synced, but write even if sync is pending

* Fix compilation

* Fix checkstyle, test

* Revert some extra changes

* Add 3 modes of cache usage

* Move enum to SegmentMetadataCache

* Run tests in all 3 cache modes

* Fix docs and IT configs

* Fix config binding

* Remove forbidden api

* Fix typos, docs and enum casing

* Fix doc

* Add json, array, aggregation function tests to quidem (#17842)

This PR adds the sql-native portion of the json, array, and aggregation function tests to quidem.  It adds a total of 9965 queries, with 6752 positive tests and 3213 negative tests.

* Optionally include Content-Disposition header in statement results API response (#17840)

Adds support for an optional filename query parameter to the /druid/v2/sql/statements/{queryId}/results API. When provided, the response will include a header Content-Disposition: attachment; filename="{filename}", which will instruct a web browser to save the response as a file rather than displaying it inline.

This save-as-attachment behavior could be achieved by adding a "download" attribute to the results link, but this only works for same-origin URLs (as in the Web Console). If the UI origin is different from the Druid API origin, browsers will ignore the attribute and serve the results inline, which is poor UX for files that are potentially very large.

For the sake of consistency, all successful responses in SqlStatementResource.doGetResults may include this header, even if there are no results.
Release note

Improved: The "Get query results" statements API supports an optional filename query parameter. When provided, the response will instruct web browsers to save the results as a file instead of showing them inline (via the Content-Disposition header).

* Web console: download follow up (#17845)

* set filename

* update download button

* added markdown support

* add test

* better download

* fix TSV

* better download behaviour and tests

* always show download all button

* Fix flaky unit tests in SegmentBootstrapperTest and KinesisIndexTaskTest (#17841)

Changes:
- Fix flakiness in SegmentBootstrapperTest
- Make TestSegmentCacheManager thread safe by moving from ArrayList to CopyOnWriteArrayList
- Modify assertions to disregard list ordering since order of list modifications is not always deterministic
- Fix flaky KinesisIndexTask tests.

* Web console: responding to user feedback about the explore view and fixing bugs (#17844)

* better debounce

* better cumpose filter

* hook up preview filters

* better stack handling

* fix some props

* refactor stack to facet

* fix hover part 1

* line hover part 2

* start adding moduleWhere

* info popover

* add filter icon

* toggle button

* module filter bar

* update TestSegmentCacheManager

* revert some style changes

* validate datasource in CachingClusteredClient as well

* fix build failure and update style

* changes

* add inlineds test

* add sanity check on segment

* inject policy enforcer

* add PolicyEnforcer binding in MSQTestBase

* add check in SinkQuerySegmentWalker

* more tests in realtime server

* revert config change in examples

* revert config change in integration test config

* more tests in msq

* another test for unnest in msq

* add support for policy from extension

* more test

* refactor MSQTaskQueryMakerTest to use an instance of MSQTaskQueryMaker

* Add test for JoinDataSource

* add policyEnforcer to withPolicies, and validate segment after segment mapping

* fix binding and test

* add policy module

* mock planner toolbox

* revert some injection

* add test for stream appenderator

* update PolicyEnforcer to take ReferenceCountingSegment as param

* update to QueryLifecycleTest

* update to SqlTestFramework

* pass enforcer to BroadcastJoinSegmentMapFnProcessor and add test. PolicyEnforcer should also deal with multiple layer wrapped segments/

* ReferenceCountingSegment is not allowed to wrap with a SegmentReference, and PolicyEnforcer now validates all segments, remove test cases for inline/lookup.

* moving ReferenceCountingSegment to another pr

* Revert "Merge remote-tracking branch 'cecemei/debug' into policy"

This reverts commit 25ffb7c, reversing
changes made to 1e6632f.

---------

Signed-off-by: Emmanuel Ferdman <emmanuelferdman@gmail.com>
Co-authored-by: Virushade <70288012+GWphua@users.noreply.github.com>
Co-authored-by: Eyal Yurman <eyal.yurman@gmail.com>
Co-authored-by: Kashif Faraz <kashif.faraz@gmail.com>
Co-authored-by: Frank Chen <frank.chen021@outlook.com>
Co-authored-by: Chetan Patidar <122344823+chetanpatidar26@users.noreply.github.com>
Co-authored-by: aho135 <ash023@ucsd.edu>
Co-authored-by: Adithya Chakilam <35785271+adithyachakilam@users.noreply.github.com>
Co-authored-by: Vadim Ogievetsky <vadim@ogievetsky.com>
Co-authored-by: Misha <mikhailsviatohorof@gmail.com>
Co-authored-by: sviatahorau <mikhail.sviatahorau@deep.bi>
Co-authored-by: Benedict Jin <asdf2014@apache.org>
Co-authored-by: Clint Wylie <cwylie@apache.org>
Co-authored-by: Katya Macedo <38017980+ektravel@users.noreply.github.com>
Co-authored-by: Victoria Lim <vtlim@users.noreply.github.com>
Co-authored-by: Zoltan Haindrich <kirk@rxd.hu>
Co-authored-by: Gian Merlino <gianmerlino@gmail.com>
Co-authored-by: Emmanuel Ferdman <emmanuelferdman@gmail.com>
Co-authored-by: Om Kenge <88768848+omkenge@users.noreply.github.com>
Co-authored-by: Karan Kumar <karankumar1100@gmail.com>
Co-authored-by: Lars Francke <lars.francke@stackable.tech>
Co-authored-by: Adarsh Sanjeev <adarshsanjeev@gmail.com>
Co-authored-by: Akshat Jain <akjn11@gmail.com>
Co-authored-by: Andy Tsai <61856143+weishiuntsai@users.noreply.github.com>
Co-authored-by: Maytas Monsereenusorn <maytasm@apache.org>
Co-authored-by: jtuglu-netflix <jtuglu@netflix.com>
Co-authored-by: Lucas Capistrant <capistrant@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area - Documentation Area - Ingestion Needs web console change Backend API changes that would benefit from frontend support in the web console

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants