Skip to content

feat: Don't allow to disable native orchestrator#10919

Merged
ovr merged 7 commits into
masterfrom
feat/remove-native-orchestrator-flag
May 21, 2026
Merged

feat: Don't allow to disable native orchestrator#10919
ovr merged 7 commits into
masterfrom
feat/remove-native-orchestrator-flag

Conversation

@ovr
Copy link
Copy Markdown
Member

@ovr ovr commented May 20, 2026

While I’m working on improving Cube’s performance and moving toward columnar-based results, I don’t want to keep maintaining the legacy plain-JS deserialization path. Let’s remove it entirely, the native orchestrator is already battle-tested.

@ovr ovr requested review from a team and keydunov as code owners May 20, 2026 16:18
@github-actions github-actions Bot added javascript Pull requests that update Javascript code data source driver labels May 20, 2026
@ovr ovr changed the title feat: Remove CUBEJS_TESSERACT_ORCHESTRATOR variable (always enabled b… feat: Don't allow to disable native orchestrator May 20, 2026
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 20, 2026

Claude finished @ovr's task in 1m 15s —— View job


PR #10919 Review: Remove CUBEJS_TESSERACT_ORCHESTRATOR variable

  • Gather context and review PR diff
  • Review code quality and correctness
  • Check for security, performance, and testing concerns
  • Post comprehensive review

Overall: Clean, well-scoped removal. This PR removes the CUBEJS_TESSERACT_ORCHESTRATOR feature flag that was already defaulting to true, and hardcodes the native Rust deserialization path as the only code path. The changes are consistent across CI workflows, docs (both sites), the env config, and the runtime code.

Code Quality

Positive observations:

  • The rename from resolversresolver (singular) in WebSocketConnection.ts is a nice clarity improvement, since the variable holds a single { resolve, reject } pair.
  • The simplified message handler (lines 150–167) is much easier to follow now — the old branching logic (native path for large messages, JS flatbuffers path otherwise) is collapsed into a single try/catch with parseCubestoreResultMessage.
  • No remaining references to nativeOrchestrator or CUBEJS_TESSERACT_ORCHESTRATOR anywhere in the codebase — confirmed via grep.

Issue: Unused Imports

After removing the JS flatbuffers deserialization fallback, two imports in WebSocketConnection.ts are now dead code:

  • HttpError (line 12) — was only used in the removed HttpCommand.HttpError branch
  • HttpResultSet (line 17) — was only used in the removed HttpCommand.HttpResultSet branch

HttpCommand is still used at line 350, so that import is fine.

Fix unused imports →

Behavioral Change Worth Noting

The old code had a msg.length > 1000 threshold — messages under 1000 bytes were always deserialized via the JS flatbuffers path, even when the native orchestrator was enabled. Now all messages go through parseCubestoreResultMessage regardless of size. This is almost certainly fine (the native parser handles small messages just as well), but worth calling out as a subtle behavioral difference versus the previous "native orchestrator enabled" behavior.

Error Handling

The old JS path had explicit error cases for HttpCommand.HttpError (Cube Store query errors) and edge cases like null result sets, null rows, and undefined column names. The native parseCubestoreResultMessage in Rust now handles all of these. I verified that the native function is called with the raw message buffer and returns a ResultWrapper, so error semantics are delegated to the Rust implementation — this is fine as long as the Rust side properly surfaces Cube Store error responses (which it should, given the flag was already defaulting to true in production).

Summary

Looks good to merge with one minor cleanup: remove the HttpError and HttpResultSet unused imports.

@mintlify
Copy link
Copy Markdown
Contributor

mintlify Bot commented May 20, 2026

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
cubed3 🟢 Ready View Preview May 20, 2026, 4:20 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.94%. Comparing base (346eb56) to head (5abf737).
⚠️ Report is 11 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #10919   +/-   ##
=======================================
  Coverage   78.93%   78.94%           
=======================================
  Files         470      470           
  Lines       92862    92868    +6     
  Branches     3449     3450    +1     
=======================================
+ Hits        73304    73310    +6     
+ Misses      19054    19053    -1     
- Partials      504      505    +1     
Flag Coverage Δ
cube-backend 58.42% <ø> (-0.02%) ⬇️
cubesql 83.53% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions github-actions Bot added the rust Pull requests that update Rust code label May 21, 2026
@ovr ovr merged commit 9268376 into master May 21, 2026
120 of 122 checks passed
@ovr ovr deleted the feat/remove-native-orchestrator-flag branch May 21, 2026 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

data source driver javascript Pull requests that update Javascript code rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants