Skip to content

fix(infra): switch testnet DBs to x86#314

Merged
xDarksome merged 7 commits into
mainfrom
fix/infra-testnet-switch-dbs-to-x86
Jan 15, 2026
Merged

fix(infra): switch testnet DBs to x86#314
xDarksome merged 7 commits into
mainfrom
fix/infra-testnet-switch-dbs-to-x86

Conversation

@xDarksome

Copy link
Copy Markdown
Member

Description

ARM instances are totally unstable during data migrations. There's some issue in RocksDB on ARM making it periodically crash.

How Has This Been Tested?

Manually

Due Diligence

  • Breaking change
  • Requires a documentation update
  • Requires a e2e/integration test update

@claude

claude Bot commented Jan 15, 2026

Copy link
Copy Markdown

Claude finished @xDarksome's task in 1m 38s —— View job


Code Review

Issue 1: Commit Message Contradicts Code Change
File: infra/testnet/main.tf:135
Severity: Medium

The commit message "switch operator-b db to arm" contradicts the actual code change, which switches operator-b to x86 architecture. The change adds { cpu_arch = "x86" } to the db configuration.

Based on infra/modules/service/main.tf:59, when cpu_arch is not specified, it defaults to "arm". Therefore:

  • Before this PR: operator-b used ARM (default)
  • After this PR: operator-b uses x86 (explicit override)

The commit message should accurately describe switching FROM arm TO x86, not the reverse.

Issue 2: Inconsistent Pattern With Other Operators
File: infra/testnet/main.tf:110-149

After this change, testnet will have:

  • wallet-connect: x86 (with clear justification comment at line 112-113)
  • operator-a: ARM (default)
  • operator-b: x86 (no comment explaining rationale)
  • operator-c: ARM (default)
  • operator-d: ARM (default)

The wallet-connect operator includes a comment explaining why it uses x86 ("so we have both architectures being actively tested"). However, operator-b's switch to x86 lacks similar documentation. Given the PR description mentions ARM instability during data migrations, this critical context should be documented inline for future maintainers.

Issue 3: Incomplete Migration Strategy
Context: PR description states "ARM instances are totally unstable during data migrations"

If ARM has fundamental instability issues with RocksDB during migrations (not just testing diversity), only switching 1 of 4 operators to x86 leaves 3 operators (operator-a, operator-c, operator-d) vulnerable to the same crashes. The PR description suggests this is a reliability fix, not a testing strategy.

Consider whether all testnet DB instances should migrate to x86, or clarify if this is a gradual rollout strategy.


@xDarksome xDarksome marked this pull request as ready for review January 15, 2026 11:02

Copilot AI left a comment

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.

Pull request overview

This PR addresses RocksDB stability issues on ARM architecture by switching all testnet database instances to x86. Previously, only the wallet-connect operator used x86 for testing purposes, while other operators defaulted to ARM.

Changes:

  • Sets default database architecture to x86 for all testnet operators
  • Removes the special-case x86 override for the wallet-connect operator
  • Minor formatting cleanup in IAM policy alignment

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.

File Description
infra/testnet/main.tf Adds cpu_arch = "x86" to the shared db_config, ensuring all testnet database instances use x86 architecture; removes now-redundant override for wallet-connect operator
infra/modules/service/main.tf Aligns formatting of Effect and Action fields in S3 IAM policy statement

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@xDarksome xDarksome merged commit 0419592 into main Jan 15, 2026
19 checks passed
@xDarksome xDarksome deleted the fix/infra-testnet-switch-dbs-to-x86 branch January 15, 2026 11:06
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.

3 participants