Skip to content

Upstream: b46be6eb22dfa9ee671b07e279744d9b6a2eb7bd#741

Merged
kgyrtkirk merged 12 commits intomasterfrom
up-b46be6eb22dfa9ee671b07e279744d9b6a2eb7bd
Mar 25, 2025
Merged

Upstream: b46be6eb22dfa9ee671b07e279744d9b6a2eb7bd#741
kgyrtkirk merged 12 commits intomasterfrom
up-b46be6eb22dfa9ee671b07e279744d9b6a2eb7bd

Conversation

@kgyrtkirk
Copy link
Copy Markdown
Owner

@kgyrtkirk kgyrtkirk commented Mar 24, 2025

No description provided.

cryptoe and others added 12 commits March 18, 2025 19:39
@cryptoe
Revert "Run JDK 21 workflows with latest JDK. (apache#17694)" (apache…
2d986ac 
…#17806)

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

This reverts commit 31ede5c

* Review comments.

* Review comments.
@kfaraz
Revert "reject publishing actions with a retriable error code if a ea…
27850a7 
…rlier task is still publishing (apache#17509)"

This reverts commit aca56d6.
@kgyrtkirk
Fix unstable tests after apache#17787 and dart usage in quidem-ut (ap…
bb0c502 
…ache#17814)

* fixes

* fix cleanup
@gianm
Use "mix" shuffle spec for target size with nil clusterBy. (apache#17810
2300bb4 
)

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.
@lfrancke
Docs: Recommend using runtime property javaOptsArray instead of javaOpts
79b170b
@adarshsanjeev
Add minor checks in jetty utils (apache#17817)
565b310 
Add minor checks in jetty utils class
@Akshat-Jain
CI improvement: Leverage cancelled() instead of always() for CI jobs (a…
7c5d8bd 
…pache#17819)
@kgyrtkirk
Make MSQ tests use the same datasets as other similar tests (apache#1…
b9fe6ec 
…7818)

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.
@weishiuntsai
Add unnest tests to quidem (apache#17825)
b54c12f 
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.
@vogievetsky
Web console: show loader on aux queries (apache#17804)
d320e09 
* 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
@kfaraz
Use compaction dynamic config to enable compaction supervisors (apach…
5fa69bd 
…e#17782)

Changes
---------
- Remove runtime property object `CompactionSupervisorConfig`
- Add fields `useSupervisors` and `engine` to cluster-level compaction dynamic config
- Remove unused field `useAutoScaleSlots`
@kfaraz
Retry segment publish task actions without holding locks (apache#17816)
b46be6e 
apache#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
@kgyrtkirk kgyrtkirk self-assigned this Mar 24, 2025
github-advanced-security[bot]
github-advanced-security AI found potential problems Mar 24, 2025
View reviewed changes
Comment thread extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/exec/MSQInsertTest.java

@MethodSource("data")
@ParameterizedTest(name = "{index}:with context {0}")
public void testInsertOnFoo1NoDimensionsWithLimit(String contextName, Map<String, Object> context)

Check notice

Code scanning / CodeQL

Useless parameter Note test

The parameter 'contextName' is never used.

Copilot Autofix

AI about 1 year ago

To fix the problem, we should remove the unused parameter contextName from the method testInsertOnFoo1NoDimensionsWithLimit. This will simplify the method's interface and eliminate the unnecessary parameter, making the code cleaner and easier to maintain.

Suggested changeset 1
extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/exec/MSQInsertTest.java

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/exec/MSQInsertTest.java b/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/exec/MSQInsertTest.java
--- a/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/exec/MSQInsertTest.java
+++ b/extensions-core/multi-stage-query/src/test/java/org/apache/druid/msq/exec/MSQInsertTest.java
@@ -1610,3 +1610,3 @@
   @ParameterizedTest(name = "{index}:with context {0}")
-  public void testInsertOnFoo1NoDimensionsWithLimit(String contextName, Map<String, Object> context)
+  public void testInsertOnFoo1NoDimensionsWithLimit(Map<String, Object> context)
   {
EOF
@@ -1610,3 +1610,3 @@
@ParameterizedTest(name = "{index}:with context {0}")
public void testInsertOnFoo1NoDimensionsWithLimit(String contextName, Map<String, Object> context)
public void testInsertOnFoo1NoDimensionsWithLimit(Map<String, Object> context)
{
Copilot is powered by AI and may make mistakes. Always verify output.
Comment thread server/src/test/java/org/apache/druid/server/http/CoordinatorCompactionConfigsResourceTest.java
= 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
Loading
should be avoided because it has been deprecated.

Copilot Autofix

AI about 1 year ago

To fix the problem, we need to replace the usage of the deprecated method setCompactionTaskLimit with its recommended alternative. This involves identifying the new method that should be used and updating the test case accordingly. The replacement should maintain the existing functionality while adhering to the updated API.

In this case, we will replace the call to setCompactionTaskLimit with the new method updateClusterCompactionConfig which is not deprecated and provides the same functionality.

Suggested changeset 1
server/src/test/java/org/apache/druid/server/http/CoordinatorCompactionConfigsResourceTest.java

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/server/src/test/java/org/apache/druid/server/http/CoordinatorCompactionConfigsResourceTest.java b/server/src/test/java/org/apache/druid/server/http/CoordinatorCompactionConfigsResourceTest.java
--- a/server/src/test/java/org/apache/druid/server/http/CoordinatorCompactionConfigsResourceTest.java
+++ b/server/src/test/java/org/apache/druid/server/http/CoordinatorCompactionConfigsResourceTest.java
@@ -138,3 +138,6 @@
 
-    Response response = resource.setCompactionTaskLimit(0.5, 9, mockHttpServletRequest);
+    Response response = resource.updateClusterCompactionConfig(
+        new ClusterCompactionConfig(0.5, 9, null, defaultConfig.isUseSupervisors(), defaultConfig.getEngine()),
+        mockHttpServletRequest
+    );
     verifyStatus(Response.Status.OK, response);
EOF
@@ -138,3 +138,6 @@

Response response = resource.setCompactionTaskLimit(0.5, 9, mockHttpServletRequest);
Response response = resource.updateClusterCompactionConfig(
new ClusterCompactionConfig(0.5, 9, null, defaultConfig.isUseSupervisors(), defaultConfig.getEngine()),
mockHttpServletRequest
);
verifyStatus(Response.Status.OK, response);
Copilot is powered by AI and may make mistakes. Always verify output.
@kgyrtkirk kgyrtkirk merged commit b46be6e into master Mar 25, 2025
76 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@kgyrtkirk kgyrtkirk

Labels

Area - Batch Ingestion Area - Dependencies Area - Documentation Area - Ingestion Area - MSQ Area - Querying Area - Web Console GHA processed upstream

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

@kgyrtkirk @github-advanced-security @cryptoe @kfaraz @gianm @lfrancke @adarshsanjeev @Akshat-Jain @weishiuntsai @vogievetsky