Skip to content

refactor(biz): 修改提交查询逻辑以使用ID替代时间戳 #5584

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.

Sign up for GitHub

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

Closed
youngzil wants to merge 2 commits into from
Closed

refactor(biz): 修改提交查询逻辑以使用ID替代时间戳 #5584

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
933dfa9
refactor(biz): 修改提交查询逻辑以使用ID替代时间戳
youngzil Mar 20, 2026
c6e9374
fix(history): 使用发布ID替代时间戳进行提交历史查询以避免慢SQL
youngzil Mar 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,5 @@ target
*.orig
.flattened-pom.xml

.gradle/*
.vscode/*
1 change: 1 addition & 0 deletions CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ Release Notes.
Apollo 3.0.0

------------------
* [Fix: use release ID instead of timestamp for commit history query to avoid slow SQL](https://github.com/apolloconfig/apollo/pull/5584)
* [Fix: include super admin in hasAnyPermission semantics](https://github.com/apolloconfig/apollo/pull/5568)

------------------
Expand Down
4 changes: 2 additions & 2 deletions ...vice/src/main/java/com/ctrip/framework/apollo/adminservice/controller/ItemController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,8 @@ public List<ItemDTO> findDeletedItems(@PathVariable("appId") String appId,
releaseService.findLatestActiveRelease(appId, clusterName, namespaceName);
List<Commit> commits;
if (Objects.nonNull(latestActiveRelease)) {
commits = commitService.find(appId, clusterName, namespaceName,
latestActiveRelease.getDataChangeCreatedTime(), null);
commits =
commitService.find(appId, clusterName, namespaceName, latestActiveRelease.getId(), null);
Comment on lines +205 to +206
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 20, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify that Release and Commit entities both extend BaseEntity with independent ID sequences
echo "=== Checking Release entity ==="
ast-grep --pattern 'class Release extends BaseEntity'

echo "=== Checking Commit entity ==="
ast-grep --pattern 'class Commit extends BaseEntity'

echo "=== Checking BaseEntity ID generation strategy ==="
rg -nA3 '@GeneratedValue' --glob '**/BaseEntity.java'

Repository: apolloconfig/apollo

Length of output: 1145

🏁 Script executed:

#!/bin/bash
# Find Release and Commit entity files
echo "=== Finding Release entity ==="
fd -e java -x grep -l 'class Release' {} +

echo -e "\n=== Finding Commit entity ==="
fd -e java -x grep -l 'class Commit' {} +

echo -e "\n=== Searching for Release class definition ==="
rg -A5 'class Release' --glob '**/*.java' -m 1

echo -e "\n=== Searching for Commit class definition ==="
rg -A5 'class Commit' --glob '**/*.java' -m 1

Repository: apolloconfig/apollo

Length of output: 84254

🏁 Script executed:

#!/bin/bash
# View Release entity class
echo "=== Release.java ==="
cat -n apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/Release.java | head -50

# View Commit entity class
echo -e "\n=== Commit.java ==="
cat -n apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/Commit.java | head -50

# Find the CommitService.find method
echo -e "\n=== CommitService.find method ==="
rg -A20 'public.*find' apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/CommitService.java

# View ItemController around lines 205-206
echo -e "\n=== ItemController.java lines 200-210 ==="
cat -n apollo-adminservice/src/main/java/com/ctrip/framework/apollo/adminservice/controller/ItemController.java | sed -n '200,210p'

Repository: apolloconfig/apollo

Length of output: 5608

🏁 Script executed:

#!/bin/bash
# Examine the CommitRepository to understand the actual query
echo "=== CommitRepository findByAppIdAndClusterNameAndNamespaceNameAndIdGreaterThanEqualOrderByIdDesc ==="
rg -B5 -A10 'IdGreaterThanEqualOrderByIdDesc' apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/repository/CommitRepository.java

Repository: apolloconfig/apollo

Length of output: 1027

Critical: Cross-entity ID comparison produces incorrect results.

Release.id and Commit.id are generated from independent auto-increment sequences (both entities extend BaseEntity which uses GenerationType.IDENTITY). Filtering commits where commit.id >= release.id has no meaningful correlation to the release timing.

For example, if Release has ID 5000 but relevant Commit records have IDs 100-200, this query will return zero results even though those commits occurred after the release.

The original timestamp-based approach was semantically correct. To fix the performance issue, consider:

  1. Add an index on Commit.DataChange_LastTime and keep the timestamp-based query, or
  2. Store a reference (e.g., store lastCommitId on Release at release time and filter using that), or
  3. Query the Commit table to find the minimum Commit ID with dataChangeCreatedTime >= latestActiveRelease.getDataChangeCreatedTime(), then use that Commit ID for subsequent filtering.
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@apollo-adminservice/src/main/java/com/ctrip/framework/apollo/adminservice/controller/ItemController.java`
around lines 205 - 206, The code incorrectly filters commits by comparing
commit.id >= latestActiveRelease.getId() (see commitService.find call and use of
latestActiveRelease.getId()), but Release.id and Commit.id are independent
sequences; instead change to a timestamp-based filter: use
latestActiveRelease.getDataChangeCreatedTime() to query commits (or first query
Commit to find the minimal commit ID where dataChangeCreatedTime >=
latestActiveRelease.getDataChangeCreatedTime() and then pass that commitId into
commitService.find), and/or add an index on Commit.DataChange_LastTime to
preserve performance; update the call sites and commitService.find
signature/implementation as needed to accept a timestamp or startingCommitId
(referencing commitService.find and
latestActiveRelease.getDataChangeCreatedTime) so commits are selected by time
correlation rather than cross-entity IDs.

} else {
commits = commitService.find(appId, clusterName, namespaceName, null);
}
Expand Down
6 changes: 2 additions & 4 deletions apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/repository/CommitRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

import com.ctrip.framework.apollo.biz.entity.Commit;

import java.util.Date;
import org.springframework.data.domain.Pageable;
import org.springframework.data.jpa.repository.Modifying;
import org.springframework.data.jpa.repository.Query;
Expand All @@ -31,9 +30,8 @@ public interface CommitRepository extends JpaRepository<Commit, Long> {
List<Commit> findByAppIdAndClusterNameAndNamespaceNameOrderByIdDesc(String appId,
String clusterName, String namespaceName, Pageable pageable);

List<Commit> findByAppIdAndClusterNameAndNamespaceNameAndDataChangeLastModifiedTimeGreaterThanEqualOrderByIdDesc(
String appId, String clusterName, String namespaceName, Date dataChangeLastModifiedTime,
Pageable pageable);
List<Commit> findByAppIdAndClusterNameAndNamespaceNameAndIdGreaterThanEqualOrderByIdDesc(
String appId, String clusterName, String namespaceName, Long id, Pageable pageable);

@Modifying
@Query("update Commit set isDeleted = true, "
Expand Down
9 changes: 4 additions & 5 deletions apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/service/CommitService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

import com.ctrip.framework.apollo.biz.entity.Commit;
import com.ctrip.framework.apollo.biz.repository.CommitRepository;
import java.util.Date;
import org.springframework.data.domain.Pageable;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
Expand Down Expand Up @@ -53,11 +52,11 @@ public List<Commit> find(String appId, String clusterName, String namespaceName,
clusterName, namespaceName, page);
}

public List<Commit> find(String appId, String clusterName, String namespaceName,
Date lastModifiedTime, Pageable page) {
public List<Commit> find(String appId, String clusterName, String namespaceName, Long id,
Pageable page) {
return commitRepository
.findByAppIdAndClusterNameAndNamespaceNameAndDataChangeLastModifiedTimeGreaterThanEqualOrderByIdDesc(
appId, clusterName, namespaceName, lastModifiedTime, page);
.findByAppIdAndClusterNameAndNamespaceNameAndIdGreaterThanEqualOrderByIdDesc(appId,
clusterName, namespaceName, id, page);
}

public List<Commit> findByKey(String appId, String clusterName, String namespaceName, String key,
Expand Down
43 changes: 18 additions & 25 deletions ...src/test/java/com/ctrip/framework/apollo/biz/service/NamespaceServiceIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,7 @@
import com.ctrip.framework.apollo.common.entity.AppNamespace;

import com.ctrip.framework.apollo.common.exception.ServiceException;
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.Arrays;
import java.util.Date;
import java.util.HashSet;
import org.junit.Assert;
import org.junit.Test;
Expand Down Expand Up @@ -124,28 +121,24 @@ public void testDeleteNamespace() {
@Test
@Sql(scripts = "/sql/namespace-test.sql", executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD)
@Sql(scripts = "/sql/clean.sql", executionPhase = Sql.ExecutionPhase.AFTER_TEST_METHOD)
public void testGetCommitsByModifiedTime() throws ParseException {
String format = "yyyy-MM-dd HH:mm:ss";
SimpleDateFormat simpleDateFormat = new SimpleDateFormat(format);

Date lastModifiedTime = simpleDateFormat.parse("2020-08-22 09:00:00");

List<Commit> commitsByDate = commitService.find(commitTestApp, testCluster,
testPrivateNamespace, lastModifiedTime, null);

Date lastModifiedTimeGreater = simpleDateFormat.parse("2020-08-22 11:00:00");
List<Commit> commitsByDateGreater = commitService.find(commitTestApp, testCluster,
testPrivateNamespace, lastModifiedTimeGreater, null);


Date lastModifiedTimePage = simpleDateFormat.parse("2020-08-22 09:30:00");
List<Commit> commitsByDatePage = commitService.find(commitTestApp, testCluster,
testPrivateNamespace, lastModifiedTimePage, PageRequest.of(0, 1));

assertEquals(1, commitsByDate.size());
assertEquals(0, commitsByDateGreater.size());
assertEquals(1, commitsByDatePage.size());

public void testGetCommitsByModifiedTime() {
// namespace-test.sql has 1 commit for (commitTestApp, default, application)
// find(..., id, page) returns commits with id >= given id for the namespace
List<Commit> allCommits = commitService.find(commitTestApp, testCluster,
testPrivateNamespace, PageRequest.of(0, 10));
assertEquals(1, allCommits.size());
Long commitId = allCommits.get(0).getId();

List<Commit> commitsById = commitService.find(commitTestApp, testCluster,
testPrivateNamespace, commitId, null);
List<Commit> commitsByIdGreater = commitService.find(commitTestApp, testCluster,
testPrivateNamespace, commitId + 1, null);
List<Commit> commitsByIdPage = commitService.find(commitTestApp, testCluster,
testPrivateNamespace, commitId, PageRequest.of(0, 1));

assertEquals(1, commitsById.size());
assertEquals(0, commitsByIdGreater.size());
assertEquals(1, commitsByIdPage.size());
Comment on lines +124 to +141
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 20, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix Spotless formatting violations and rename test method.

  1. Formatting: The pipeline failed due to Spotless formatting violations. Run mvn spotless:apply before committing.

  2. Misleading name: The method testGetCommitsByModifiedTime no longer tests timestamp-based filtering. Rename it to reflect the current behavior, e.g., testGetCommitsByIdFilter or testFindCommitsByIdThreshold.

As per coding guidelines, "Run ./mvnw spotless:apply to format code and must be run before opening a PR".

🧰 Tools
🪛 GitHub Actions: code style check

[error] 124-131: Spotless Java formatting check failed in spotless:2.43.0:check for project apollo-biz. The following files had format violations, including incorrect line breaks/indentation in List assignments (e.g., allCommits, commitsById, commitsByIdGreater). Run 'mvn spotless:apply' to fix.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@apollo-biz/src/test/java/com/ctrip/framework/apollo/biz/service/NamespaceServiceIntegrationTest.java`
around lines 124 - 141, The test method name and formatting need fixing: run the
formatter (mvn spotless:apply or ./mvnw spotless:apply) and commit the changes,
then rename the test method
NamespaceServiceIntegrationTest.testGetCommitsByModifiedTime to a name that
matches its behavior (e.g., testGetCommitsByIdFilter or
testFindCommitsByIdThreshold) and update any references to that method; ensure
the method body (calls to commitService.find and assertions) remains unchanged
and that the class compiles after the rename.

}


Expand Down
Loading