Repository navigation
Conversation
Remove the non-prepared numeric row API and its obsolete tests. Extend the opt-in single-row benchmark with partial-column workloads, latency, write-stage timing and process resource sampling.
jt2594838
left a comment
jt2594838
left a comment
There was a problem hiding this comment.
Author rationale notes (1-50 of 201), anchored to the corresponding changes. Validation and benchmark limits are in the PR description.
| } | ||
|
|
||
| @Override | ||
| public DataNodeConfig setMetricPrometheusReporterAsyncUpdate(boolean asyncUpdate) { |
There was a problem hiding this comment.
MppDataNodeConfig.java:60 — Expose the existing Prometheus update mode to local IT environments so timing samples can request fresh metrics at round boundaries instead of inheriting asynchronous reporter lag.
| } | ||
|
|
||
| @Override | ||
| public DataNodeConfig setMetricPrometheusReporterAsyncUpdate(boolean asyncUpdate) { |
There was a problem hiding this comment.
RemoteDataNodeConfig.java:32 — Keep the remote environment compatible with the shared test configuration interface. A remote IT does not own server configuration, so this setter follows the other remote no-op setters.
| public interface DataNodeConfig { | ||
| DataNodeConfig setMetricReporterType(List<String> metricReporterTypes); | ||
|
|
||
| DataNodeConfig setMetricPrometheusReporterAsyncUpdate(boolean asyncUpdate); |
There was a problem hiding this comment.
DataNodeConfig.java:28 — Add the test-environment configuration contract needed for symmetric write-stage sampling; this selects an existing server setting, rather than introducing a production configuration key.
| public static final String NOT_DIRECT_CHILD_OF_TEMPLATE = | ||
| "It is not a direct child of the template: "; | ||
|
|
||
| public static final String EXCEPTION_UNSUPPORTED_NUMERIC_TYPE_06DD3044 = "Unsupported numeric type"; |
There was a problem hiding this comment.
ISessionMessages.java:34 — Provide English diagnostics for numeric metadata, payload or handle validation in the existing message catalog. Both locales use matching keys and format placeholders; default and Chinese compilation passed.
| public static final String NOT_DIRECT_CHILD_OF_TEMPLATE = | ||
| "这不是该模板的直接子节点："; | ||
|
|
||
| public static final String EXCEPTION_UNSUPPORTED_NUMERIC_TYPE_06DD3044 = "不支持的数值类型"; |
There was a problem hiding this comment.
ISessionMessages.java:34 — Provide Chinese diagnostics for numeric metadata, payload or handle validation in the existing message catalog. Both locales use matching keys and format placeholders; default and Chinese compilation passed.
| int insert(InsertRowNode insertRowNode); | ||
|
|
||
| int insertAlignedRow(InsertRowNode insertRowNode); | ||
| int insertAlignedRow(AbstractInsertRowNode insertRowNode); |
There was a problem hiding this comment.
IMemTable.java:110 — Generalize the aligned-row MemTable boundary so both boxed and numeric rows share the storage interface; implementation dispatch selects the representation-specific insertion.
| * @param insertRowNode physical plan of insertion | ||
| */ | ||
| public void insert(InsertRowNode insertRowNode, long[] infoForMetrics) | ||
| public void insert(AbstractInsertRowNode insertRowNode, long[] infoForMetrics) |
There was a problem hiding this comment.
TsFileProcessor.java:318 — Carry the common row contract through TsFileProcessor insertion and replica-set cleanup. Numeric rows still use the existing write/WAL/flush sequence, with primitive-aware accounting instead of constructing boxed values.
| if (value instanceof IMemTable) { | ||
| this.type = WALEntryType.MEMORY_TABLE_SNAPSHOT; | ||
| } else if (value instanceof InsertRowNode) { | ||
| } else if (value instanceof AbstractInsertRowNode) { |
There was a problem hiding this comment.
WALEntry.java:68 — Recognize both row implementations in WAL entry classification and deserialization. The row entry envelope stays shared, while the embedded plan discriminator identifies the numeric payload.
| break; | ||
| case INSERT_ROW_NODE: | ||
| ((InsertRowNode) value).serializeToWAL(buffer, encodedSearchIndex); | ||
| ((AbstractInsertRowNode) value).serializeToWAL(buffer, encodedSearchIndex); |
There was a problem hiding this comment.
WALInfoEntry.java:105 — Serialize either row representation using the WAL entry stored search index. Passing the captured index prevents later plan mutation from changing the consensus position persisted in the entry.
|
|
||
| /** Log InsertRowNode. */ | ||
| WALFlushListener log(long memTableId, InsertRowNode insertRowNode); | ||
| WALFlushListener log(long memTableId, AbstractInsertRowNode insertRowNode); |
There was a problem hiding this comment.
IWALNode.java:41 — Allow WAL callers to log the common row abstraction, so numeric values reach their own serializer without a conversion to an ordinary row.
jt2594838
left a comment
jt2594838
left a comment
There was a problem hiding this comment.
Author rationale notes (51-100 of 201), anchored to the corresponding changes. Validation and benchmark limits are in the PR description.
|
|
||
| @Override | ||
| public WALFlushListener log(long memTableId, InsertRowNode insertRowNode) { | ||
| public WALFlushListener log(long memTableId, AbstractInsertRowNode insertRowNode) { |
There was a problem hiding this comment.
WALFakeNode.java:56 — Keep the disabled/fake WAL implementation consistent with the widened row contract, preserving the existing flush-listener behavior for numeric writes.
|
|
||
| @Override | ||
| public WALFlushListener log(long memTableId, InsertRowNode insertRowNode) { | ||
| public WALFlushListener log(long memTableId, AbstractInsertRowNode insertRowNode) { |
There was a problem hiding this comment.
WALNode.java:225 — Accept primitive and boxed rows through the same WAL enqueue path. Existing durability and listener handling remain shared.
| long minTimeInNode; | ||
| if (node instanceof InsertRowNode) { | ||
| minTimeInNode = ((InsertRowNode) node).getTime(); | ||
| if (node instanceof AbstractInsertRowNode row) { |
There was a problem hiding this comment.
TsFilePlanRedoer.java:100 — Replay either row representation using the common timestamp and aligned insertion contract. Recovered numeric plans can populate the MemTable directly instead of losing their representation during recovery.
| return NoopOperationQuota.get(); | ||
| } | ||
| switch (s.getType()) { | ||
| case INSERT_NUMERIC_ROW: |
There was a problem hiding this comment.
DataNodeThrottleQuotaManager.java:84 — Classify prepared numeric inserts as writes for throttle checks. Preparing metadata must not create a path that bypasses execution-time write quotas.
| } | ||
| final StatementType statementType = s.getType(); | ||
| switch (statementType) { | ||
| case INSERT_NUMERIC_ROW: |
There was a problem hiding this comment.
DefaultOperationQuota.java:106 — Estimate numeric write size from non-null typed slots, including actual STRING TAG sizes. This supplies quota accounting without materializing a boxed row or charging null slots as values.
| } | ||
|
|
||
| @Override | ||
| public TSStatus closePreparedNumericRow(long sessionId, long preparedId) { |
There was a problem hiding this comment.
ClientRPCServiceImpl.java:2675 — Validate session ownership on explicit close and reuse the registry idempotent removal path. A caller cannot release another physical session prepared handle.
| } | ||
|
|
||
| InsertRowStatement statement = StatementGenerator.createStatement(req); | ||
| InsertBaseStatement statement = StatementGenerator.createStatement(req); |
There was a problem hiding this comment.
ClientRPCServiceImpl.java:2699 — Generalize existing record RPC callers to the generator common insert type while retaining their table/tree authorization paths. This lets the shared execution code handle the new representation without a boxed-row assumption.
| private long time; | ||
| private NumericRowValues numericValues; | ||
|
|
||
| public RelationalInsertNumericRowNode(PlanNodeId id) { |
There was a problem hiding this comment.
RelationalInsertNumericRowNode.java:67 — Keep time, metadata and NumericRowValues directly in the physical plan. The common row API supports downstream consumers without introducing an Object[] field alongside primitive storage.
| } | ||
|
|
||
| @Override | ||
| public IDeviceID getDeviceID() { |
There was a problem hiding this comment.
RelationalInsertNumericRowNode.java:157 — Construct table device identity from TAG values while keeping failed/missing column state explicit. TAG identity and FIELD validity must remain correct after schema normalization and partial-column writes.
| } | ||
|
|
||
| @Override | ||
| public List<WritePlanNode> splitByPartition(IAnalysis analysis) { |
There was a problem hiding this comment.
RelationalInsertNumericRowNode.java:228 — Route the single row to its data partition and preserve the normal write-plan interfaces. A numeric representation changes payload handling, not partition ownership.
jt2594838
left a comment
jt2594838
left a comment
There was a problem hiding this comment.
Author rationale notes (101-150 of 201), anchored to the corresponding changes. Validation and benchmark limits are in the PR description.
| } | ||
|
|
||
| @Override | ||
| public TimeValuePair getLastCacheValue(int index) { |
There was a problem hiding this comment.
RelationalInsertNumericRowNode.java:271 — Convert individual values only at the last-cache boundary. This preserves cache behavior while avoiding whole-row boxing during ordinary storage insertion.
| } | ||
| } | ||
|
|
||
| private int bodySize() { |
There was a problem hiding this comment.
RelationalInsertNumericRowNode.java:291 — Use one numeric plan layout for size calculation and buffer/stream/WAL serialization, preserving metadata, nulls and primitive bits. A distinct plan discriminator makes the representation explicit to recovery and consensus readers.
| } | ||
|
|
||
| @Override | ||
| public void serializeToWAL(IWALByteBufferView buffer, long encodedSearchIndex) { |
There was a problem hiding this comment.
RelationalInsertNumericRowNode.java:345 — Serialize the WAL entry captured search index rather than relying on mutable plan state. The WAL-envelope regression checks that replication position remains stable when the node changes later.
| } | ||
| } | ||
|
|
||
| private static RelationalInsertNumericRowNode deserializeBody(ByteBuffer buffer) { |
There was a problem hiding this comment.
RelationalInsertNumericRowNode.java:434 — Reconstruct NumericRowValues directly in the matching buffer/stream readers. WAL and consensus round trips retain null TAGs, numeric bits and FIELD nulls instead of falling back to boxed row storage.
| } | ||
|
|
||
| @Override | ||
| public void validateTableSchema(Metadata metadata, MPPQueryContext context) { |
There was a problem hiding this comment.
InsertNumericRow.java:54 — Record both thrown and partial schema failures on the inner statement. The RPC layer can invalidate the prepared handle even if the coordinator converts an exception into a status.
| } | ||
|
|
||
| @Test | ||
| public void testNonStringTagRejectedBeforeRpc() throws Exception { |
There was a problem hiding this comment.
PreparedNumericRowMetadataTest.java:124 — Regression for non string tag rejected before rpc. Exercise metadata validation at the client boundary and inspect RPC arguments, so invalid layouts fail before a request while valid mixed TAG/FIELD order is preserved.
| } | ||
|
|
||
| @Test | ||
| public void testMixedColumnOrderPreservesMetadataAndValues() throws Exception { |
There was a problem hiding this comment.
PreparedNumericRowMetadataTest.java:136 — Regression for mixed column order preserves metadata and values. Exercise metadata validation at the client boundary and inspect RPC arguments, so invalid layouts fail before a request while valid mixed TAG/FIELD order is preserved.
|
|
||
| // Recommendations acknowledge this row; later rows prepare once on the leader with intact nulls. | ||
| @Test | ||
| public void testRedirectPreparesPerSessionWithoutReplaying() throws Exception { |
There was a problem hiding this comment.
PreparedNumericRowRedirectionTest.java:94 — Regression for redirect prepares per session without replaying. Use separate mocked physical sessions and captured execution counts to verify handle ownership/routing and to detect accidental replay of a successful or ambiguous write.
|
|
||
| // Disabling redirection ignores recommendations; USE must not prepare an old row in a new DB. | ||
| @Test | ||
| public void testDisabledRedirectionAndDatabaseChange() throws Exception { |
There was a problem hiding this comment.
PreparedNumericRowRedirectionTest.java:123 — Regression for disabled redirection and database change. Use separate mocked physical sessions and captured execution counts to verify handle ownership/routing and to detect accidental replay of a successful or ambiguous write.
|
|
||
| // Reconnecting a destination requires a new ID; reconnecting the owner invalidates the statement. | ||
| @Test | ||
| public void testReconnectedSessionsNeverReuseOldIds() throws Exception { |
There was a problem hiding this comment.
PreparedNumericRowRedirectionTest.java:140 — Regression for reconnected sessions never reuse old ids. Use separate mocked physical sessions and captured execution counts to verify handle ownership/routing and to detect accidental replay of a successful or ambiguous write.
jt2594838
left a comment
jt2594838
left a comment
There was a problem hiding this comment.
Final author rationale notes (151-201 of 201), anchored to the corresponding changes. Validation and benchmark limits are in the PR description.
|
|
||
| // A failed redirected execution is propagated once, and all handles close on a server error. | ||
| @Test | ||
| public void testFailureDoesNotReplayOrReviveStatement() throws Exception { |
There was a problem hiding this comment.
PreparedNumericRowRedirectionTest.java:160 — Regression for failure does not replay or revive statement. Use separate mocked physical sessions and captured execution counts to verify handle ownership/routing and to detect accidental replay of a successful or ambiguous write.
|
|
||
| // A preexisting device route is used immediately, including a device with a null TAG. | ||
| @Test | ||
| public void testExistingRouteAndNullTag() throws Exception { |
There was a problem hiding this comment.
PreparedNumericRowRedirectionTest.java:184 — Regression for existing route and null tag. Use separate mocked physical sessions and captured execution counts to verify handle ownership/routing and to detect accidental replay of a successful or ambiguous write.
| } | ||
|
|
||
| @Test | ||
| public void testClosedWrapperCannotUseReborrowedSession() throws Exception { |
There was a problem hiding this comment.
PreparedNumericRowPoolTest.java:83 — Regression for closed wrapper cannot use reborrowed session. Exercise a borrowed wrapper lifecycle with mocked transport failures, so a returned session cannot be reused through an old wrapper or returned to the pool twice.
| } | ||
|
|
||
| @Test | ||
| public void testPrepareTransportFailureDiscardsSession() throws Exception { |
There was a problem hiding this comment.
PreparedNumericRowPoolTest.java:117 — Regression for prepare transport failure discards session. Exercise a borrowed wrapper lifecycle with mocked transport failures, so a returned session cannot be reused through an old wrapper or returned to the pool twice.
| } | ||
|
|
||
| @Test | ||
| public void testExecuteTransportFailureDiscardsSession() throws Exception { |
There was a problem hiding this comment.
PreparedNumericRowPoolTest.java:139 — Regression for execute transport failure discards session. Exercise a borrowed wrapper lifecycle with mocked transport failures, so a returned session cannot be reused through an old wrapper or returned to the pool twice.
| return result; | ||
| } | ||
|
|
||
| private static void printPerformanceBreakdown( |
There was a problem hiding this comment.
IoTDBTableModelSessionIT.java:853 — Report per-stage deltas for the measured write window so client request time can be compared with server work. Sampling is symmetric, and the PR benchmark numbers explicitly disclose profiling overhead.
| return sorted[Math.max(0, (int) Math.ceil(quantile * sorted.length) - 1)]; | ||
| } | ||
|
|
||
| private static WritePerformance runConcurrentWrites( |
There was a problem hiding this comment.
IoTDBTableModelSessionIT.java:1039 — Coordinate writers around a shared measurement interval and record per-request durations as well as row count. This separates throughput from individual request latency and keeps worker setup outside measured requests.
| } | ||
| } | ||
|
|
||
| private static RequestLatency summarizeRequestLatencies(long[] requestNanos) { |
There was a problem hiding this comment.
IoTDBTableModelSessionIT.java:1017 — Compute latency distribution from the recorded requests, including percentile rank selection. Throughput alone can hide slow individual requests under concurrency.
| IDeviceID deviceId = insertRowNode.getDeviceID(); | ||
| TSDataType[] dataTypes = insertRowNode.getDataTypes(); | ||
| Object[] values = insertRowNode.getValues(); | ||
| Object values = insertRowNode.getValuesForMemoryAccounting(); |
There was a problem hiding this comment.
TsFileProcessor.java:854 — Make row memory estimation accept either NumericRowValues or existing object arrays. Binary-value accounting remains specific to object-backed binary columns, avoiding both boxing and invalid array casts for numeric FIELDs.
| && valuesOrColumns != null | ||
| && index < valuesOrColumns.length | ||
| && valuesOrColumns[index] != null; | ||
| && (valuesOrColumns instanceof NumericRowValues row |
There was a problem hiding this comment.
TsFileProcessor.java:1386 — Use explicit numeric-row null/presence checks for aligned memory accounting. Missing or null slots must not allocate/count value arrays as if they contained actual FIELD data.
jt2594838
left a comment
jt2594838
left a comment
There was a problem hiding this comment.
CI dependency declaration fix; local validation passed.
| </dependency> | ||
| <dependency> | ||
| <groupId>org.powermock</groupId> | ||
| <artifactId>powermock-reflect</artifactId> |
There was a problem hiding this comment.
Declare powermock-reflect directly with test scope because ClientRPCServiceImplPreparedNumericRowTest imports Whitebox. It previously arrived only transitively through PowerMock, which allowed compilation but failed the strict dependency:analyze-only check in CI. The parent POM continues to manage version 2.0.9; the dependency is excluded from the runtime package. Verified locally: strict dependency analysis and Spotless passed, with all 5 focused RPC tests passing.
1 participant
Description
Add prepared numeric single-row writes to the table Session API. Applications can prepare column metadata once and repeatedly send STRING TAG values and fixed-width numeric FIELD values, with an optional FIELD null bitmap.
Public API and behavior
ITableSession.prepareInsertNumericRow(...)andexecutePreparedInsertNumericRow(...), including the nullable overload, to direct and pooled table sessions.long[]; FLOAT/DOUBLE carry raw IEEE 754 bits. TAGs useString[]and may be null or empty.Write path and coverage
Preserve primitive numeric values through planning, MemTable insertion, WAL serialization/recovery and consensus replication. Extend Pipe and subscription adapters to consume the new row representation. The final API exposes only prepared numeric writes. Normalize integer DATE values to LocalDate at the subscription Tablet boundary, preserving DATE writes with the current TsFile Type API.
Add opt-in benchmarks comparing prepared writes with one-row Tablets, including random-device partial-column writes, request latency, stage timing, network/disk I/O and peak CPU/RSS sampling. Benchmarks remain disabled unless explicitly enabled. The array pool retains its original behavior and has no new switch.
Validation
fea5bd2490a8, covering client metadata/buffer reuse/redirection/pooling, server schema versions and handle cleanup, numeric conversion, MemTable/TVList insertion, WAL/consensus serialization and Pipe/subscription adapters.distribution. All Maven commands used-Drat.skip=true.Pipe/IoTConsensus cluster integration tests were not rerun during the final review; the focused adapter and serialization tests above passed.
This PR has:
Key changed/added classes (or packages if there are too many classes) in this PR
ITableSession,NumericRowPreparedStatement,TableSession,SessionConnection,TableSessionWrapperPreparedNumericRowInfo,InsertNumericRowStatement,RelationalInsertNumericRowNode,NumericRowValuesIoTDBTableModelSessionIT,IoTDBPipeNumericRowITand performance resource trackers