Repository navigation
perf(admin): index operation log queries (#6675) - #7404
Conversation
|
CI follow-up: |
|
Storage E2E follow-up for the previous head ( |
|
Follow-up on the current-head CI run: |
|
HTTP integration follow-up: the current-head |
|
Storage E2E update for the current head: Docker image build passed, and H2, MySQL, and openGauss storage jobs passed. PostgreSQL storage E2E is still running. These jobs exercise initialization/startup schemas; they do not execute the 2.7.1-to-2.7.2 upgrade scripts. The HTTP and Kubernetes Gateway API integration jobs are also green. |
|
Final storage matrix update for the current head: PostgreSQL storage E2E passed as well. The complete changed-initialization-schema matrix is now green for H2, MySQL, PostgreSQL, and openGauss; Docker image build, HTTP integration, and Kubernetes Gateway API integration also passed. The only failing required check remains |
|
Could a maintainer review the portability and scope of this change on head |
Aias00
left a comment
Aias00
left a comment
There was a problem hiding this comment.
This brings OceanBase / OpenGauss / Oracle / PostgreSQL to parity with MySQL, which already had both indexes (db/init/mysql/schema.sql:2879-2880 and db/upgrade/2.7.1-upgrade-2.7.2-mysql.sql). I checked all five db/init dialects plus the four 2.7.1-upgrade-2.7.2-* scripts: after this change every dialect declares an index on operation_time and a composite on (operator, operation_time), so this is parity work rather than a partial migration. Good to see init and upgrade scripts kept in step — an index that exists only in db/init silently does not exist for anyone who upgrades.
Two details worth calling out because they look wrong and aren't:
- The Oracle composite is named
idx_operation_log_oper_timewhile the others useidx_operation_log_operator_time. That is intentional: the full name is 31 characters and Oracle <= 12.1 caps identifiers at 30. Please add a short comment so nobody "fixes" it later. - OceanBase declares the indexes inline in
CREATE TABLE, og/pg use standaloneCREATE INDEXafter the PK constraint, and Oracle uses lowercasecreate index— each matches how its own file is written.
Nit: operation_record_log is append-only and (operator, operation_time) already leads with operation_time for the per-operator case, so both indexes are justified only if the admin actually lists recent operations across all operators. If it does, keep both; if not, the composite alone covers it and you save write cost on a hot table.
Aias00
left a comment
Aias00
left a comment
There was a problem hiding this comment.
Reviewed current head 6ccd107c30f2.
The PR adds operation_record_log indexes for the dialects missing them and keeps Oracle index names within its identifier limit. MySQL already contains the same logical indexes in init and upgrade scripts at head.
Validation: init/upgrade SQL reviewed across dialects; native database execution was not run locally.
GitHub still reports failing checks: build (shenyu-integrated-test-http), rat_check, pr_build. This review does not claim those checks have passed.
|
Hi @sunnysabor, this PR currently has merge conflicts with the |
|
Resolved the conflicts and rebased onto current master The conflicts came from #7529/#7530 removing the separate OceanBase/openGauss initialization files. I kept those deletions: OceanBase inherits the existing MySQL indexes, while openGauss inherits the PostgreSQL indexes added here. All four dialect-specific upgrade scripts retain the index additions; Oracle retains the short name and explanatory comment. Validation: 29 native-SQL-matrix script tests and 11 migration script tests passed; root Maven validate/Checkstyle passed; index-definition/Oracle identifier checks and |
6ccd107 to
78a5db7
Compare
78a5db7 to
b860917
Compare
|
Rebased again onto current master 1d599d9; the new head is b860917. The new conflicts were at the upgrade-script append points: I preserved upstream data_permission deduplication/unique-index migrations and retained this PR's operation-log indexes, including the Oracle-compatible identifier. The complete branch diff remains six SQL files with no Markdown. Validation: 31 native-SQL-matrix script unit tests and 11 migration script tests pass; root Maven validate reports zero Checkstyle violations; git diff --check passes. These script tests are not native database execution. I also inspected the previous head's failed openGauss storage job: https://github.com/apache/shenyu/actions/runs/38584449453/job/115990129091 . Compose reports shenyu-admin unhealthy; Admin fails during integrationDataSourceInitializer startup, with CannotGetJdbcConnectionException -> org.opengauss.util.PSQLException -> EOFException. The database log includes successful CREATE INDEX output, with no SQL ERROR/FATAL shown. This establishes the observed startup/connection failure, not a proven transient-infrastructure root cause or a passing E2E run. The rebased head will receive fresh hosted checks. |
Aias00
left a comment
Aias00
left a comment
There was a problem hiding this comment.
Summary
Addresses #6675. This PR adds missing indexes on operation_record_log(operation_time) and operation_record_log(operator, operation_time) across the Oracle and shared PostgreSQL/openGauss initialization schemas, plus the OceanBase, openGauss, Oracle, and PostgreSQL 2.7.1→2.7.2 upgrade scripts. This keeps time-range and username-filtered operation-log queries indexed on every supported database. OceanBase reuses the MySQL initialization schema, which already contained both indexes (verified at db/init/mysql/schema.sql:2477-2478), so only its upgrade path needs the new statements.
What the change does
db/init/oracle/schema.sql:422,424—create index idx_operation_log_timeandcreate index idx_operation_log_oper_time(the shorter name respects Oracle's 30-byte identifier limit).db/init/pg/create-table.sql:2066,2067—CREATE INDEX "idx_operation_log_time"and"idx_operation_log_operator_time"(shared PG/openGauss init).db/upgrade/2.7.1-upgrade-2.7.2-ob.sql:91,92—ALTER TABLE operation_record_log ADD INDEX(MySQL syntax, for OceanBase).db/upgrade/2.7.1-upgrade-2.7.2-og.sql:91,92,...-oracle.sql:105,107,...-pg.sql:91,92— matching index additions in each DB's dialect.
Verification
- Compared all six changed files at head (
b86091708f934aa652b1ae9daebc3362dc511256) againstmaster. grep idx_operation_logacross both head and master confirms the new index names appear only in the head version; no duplicate index name already exists in any target file (so no re-creation conflict within the scripts).- Table/column existence confirmed against the master schemas:
- Oracle
operation_record_log:operator VARCHAR2(200) not null,operation_time timestamp(3) not null(db/init/oracle/schema.sql:404-405). - PG
operation_record_log:"operator" varchar(200) NOT NULL,"operation_time" timestamp(6) NOT NULL(db/init/pg/create-table.sql:406-407). - The column lists in every new
CREATE INDEX/ALTER TABLE ADD INDEXmatch these definitions exactly.
- Oracle
- Premise check:
db/init/mysql/schema.sql:2477-2478(the schema OceanBase reuses) already definesidx_operation_log_timeandidx_operation_log_operator_time, so the OB init path is covered and only the OB upgrade script was needed. - CI: 28 successful, 0 failing on all relevant checks including
native-sql-matrix/*,agent_gateway_migrations,pr_build,check-license-header,rat_check,registry_dependencies,build (shenyu-integrated-test-*)builds. The only pending jobs are infra-level (build_local_repo,compat_java21,test_group,e2e-caseaggregator) — none red.
Notes (non-blocking)
- The
CREATE INDEX/ALTER TABLE ADD INDEXstatements are unconditional. On a fresh init/upgrade they are correct; re-running an init script against an already-initialized database (or a partial upgrade) would raise a duplicate-index error. This matches the existing convention in these scripts (e.g.,idx_meta_data_namespace_pathadded unconditionally in the same blocks), so it is consistent rather than a new defect. - Index naming diverges by dialect (
idx_operation_log_oper_timefor Oracle vsidx_operation_log_operator_timeelsewhere) purely for Oracle's 30-byte limit, as documented in the inline comment — intentional and correct.
Conclusion
APPROVE. The change is a correct, well-scoped SQL-only performance improvement, verified against master schemas, with all relevant CI green and no duplicate-index or column-name issues.
Description
Add the missing operation-time and operator/time indexes to Oracle and the shared PostgreSQL/openGauss initialization schema, plus the OceanBase, openGauss, Oracle, and PostgreSQL 2.7.1-to-2.7.2 upgrade scripts. OceanBase now reuses the MySQL initialization schema, which already contains both indexes. This keeps time-range and username-filtered operation-log queries indexed across these databases.
Rebased onto master at
1d599d90e; preserve the initialization consolidation from #7529/#7530 instead of restoring the deleted OceanBase/openGauss files. Oracle uses the shorteridx_operation_log_oper_timename for the 30-byte identifier limit. Query, pagination, API behavior, and leading-wildcard context search are unchanged. Related issue: #6675.Validation
python3 .github/scripts/test-native-sql-matrix.py— 31 tests passed (script/unit checks, not native database execution).python3 .github/scripts/test-agent-gateway-migrations.py— 11 tests passed../mvnw -B -ntp -N validate— passed, 0 Checkstyle violations (root project only).git diff --check upstream/master...HEADpassed; complete branch diff contains six SQL files and no Markdown.