DaZuiZui commented on issue #18428:
URL: https://github.com/apache/iotdb/issues/18428#issuecomment-5393622269
## Revised addendum addressing the review blockers
Thank you for reviewing the proposal against the current implementation and
for identifying these gaps. Your comments are very helpful, and I agree with
the points you raised. I also agree that the first three items are correctness
and compatibility blockers.
Where this revision conflicts with the previous proposal or addendum, this
revision takes precedence. The remaining parts of the previous proposal stay
unchanged.
### 1. Use exactly the same calendar and DST resolution as `GROUP BY TIME`
The CQ implementation will not introduce the previously proposed
preferred-anchor-offset rule for DST overlaps.
CQ occurrence and RANGE calculations will reuse the same calendar conversion
used by the current `GROUP BY TIME` month path, including the behavior of:
```java
LocalDateTime.plusMonths(...).atZone(zoneId)
```
The implementation will reuse `DateTimeUtils.calcPositiveIntervalByMonth`
and its underlying `TimeDuration.calcPositiveIntervalByMonth` behavior, or
extract that behavior into a shared helper used by both paths.
The resulting rules are:
- A DST overlap uses the earlier valid offset, matching the normal `atZone`
rule.
- A DST gap is shifted forward according to the zone transition.
- The original anchor offset is not preferred when resolving a later overlap.
- Calendar months are applied first.
- The fixed-duration component is then added as elapsed ticks.
- Fixed-only durations remain checked elapsed-tick addition.
- The host default time zone is never used.
The shared helper may be extended to support checked arithmetic and signed
internal duration vectors needed by RANGE calculation, but the existing
local-time resolution behavior must not change.
For the example from the review:
```text
Zone = America/New_York
B = 2024-01-03 01:30 -05:00
E = 10mo
target local time = 2024-11-03 01:30
```
both CQ and `GROUP BY TIME` will select the earlier overlap offset:
```text
2024-11-03 01:30 -04:00 = 05:30Z
```
Tests will assert exact instant equality between CQ-generated boundaries and
`GROUP BY TIME` boundaries for ordinary dates, 28/29/30/31-day month
transitions, month-end clamping, leap-day anchors, DST gaps, and DST overlaps.
This replaces the preferred-`anchorOffset` rule from the previous addendum.
### 2. Make the structured wire format and mixed-version rejection
enforceable
The request will carry an explicit duration encoding version together with
the structured duration values. For example:
```thrift
struct TCQDuration {
1: required i64 monthPart
2: required i64 nonMonthDuration
}
struct TCreateCQReq {
// Existing fields 1-10 remain unchanged.
11: optional i16 durationEncodingVersion
12: optional TCQDuration everyDuration
13: optional TCQDuration startOffsetDuration
14: optional TCQDuration endOffsetDuration
15: optional bool boundaryExplicit
}
```
For `durationEncodingVersion = 1`:
- `everyDuration`, `startOffsetDuration`, `endOffsetDuration`, and
`boundaryExplicit` must all be present.
- Partially structured requests are rejected.
- Unknown encoding versions are rejected.
- Structured durations are authoritative.
- All month values, fixed values, arithmetic operations, and narrowing
conversions are checked.
- A new DataNode sets the encoding version and all structured fields for
every newly created CQ, including fixed-only CQs.
A request without `durationEncodingVersion` is a legacy-protocol request.
A new ConfigNode will reject a markerless request at the CREATE ingress
instead of automatically interpreting it as fixed-only. This deliberately
prevents an old DataNode from sending a calendar duration that has already been
flattened to a legacy `i64` value.
During a rolling upgrade, this means an old DataNode cannot create a CQ
through a new ConfigNode. Existing persisted CQs are unaffected and continue to
run with their existing semantics.
Legacy representations encountered while loading existing procedures, plans,
metadata, or snapshots remain supported and are normalized as legacy
fixed-duration CQs. The rejection applies to new CQ creation at the RPC
ingress, not to recovery of existing state.
Calendar-bearing version-1 requests additionally require a hard capability
barrier:
1. Before forwarding the request, the SQL-ingress DataNode verifies that all
registered ConfigNodes and all registered DataNodes capable of accepting client
SQL support duration encoding version 1.
2. The receiving ConfigNode repeats the same verification against
authoritative cluster node information before accepting the request.
3. Any old, unknown, or unsupported node version causes the request to be
rejected with a clear compatibility error.
4. A calendar-bearing request must never be submitted while an old
ConfigNode could deserialize or apply it.
5. The same barrier applies before the new consensus plan, procedure
serialization, or CQ snapshot representation can be produced.
The existing required legacy `i64` fields remain unchanged for wire
compatibility and are populated as follows:
- When all three structured durations have `monthPart == 0`, the legacy
fields contain the exact fixed-duration values.
- The ConfigNode verifies that the legacy values and structured fixed values
are identical.
- If any effective duration has a nonzero `monthPart`, there is no valid
legacy representation.
- In that case, all three legacy duration fields are set to the invalid
sentinel value `0`.
- New readers validate the version-1 structured representation and do not
use the sentinel fields.
- The all-ConfigNode capability barrier guarantees that an old reader cannot
receive such a calendar request.
- The sentinel does not replace the reader barrier and must never be treated
as a compatibility representation.
- A calendar duration is never approximated as 28, 30, or 365 days in a
legacy field.
The following cases are rejected:
- a marker without all structured fields;
- structured fields without a supported marker;
- an unknown encoding version;
- conflicting fixed legacy and structured values;
- a calendar-bearing request while any required node capability is
unavailable;
- a new markerless CREATE request received by a new ConfigNode.
This closes the old-DataNode-to-new-ConfigNode ingress hole and defines both
the required legacy values and the hard reader barrier.
### 3. Advance `nextOccurrenceIndex` using an exact consensus CAS and
fencing token
`nextOccurrenceIndex` is defined as the first occurrence that has not yet
been durably completed or discarded.
Progress advancement will use a consensus plan containing at least:
```text
cqId
cqToken
expectedIndex
targetIndex
```
The CQ token acts as the CQ generation and fencing token. The state machine
atomically applies:
```text
if storedToken == cqToken
and nextOccurrenceIndex == expectedIndex:
nextOccurrenceIndex = targetIndex
```
The possible results are:
- If the CQ does not exist, the callback stops.
- If `cqToken` does not match, the callback is fenced and stops.
- If the stored index equals `expectedIndex`, the state machine updates it
to `targetIndex` and returns `ADVANCED`.
- If the stored index is greater than `expectedIndex`, the callback is stale
or duplicated. The state machine returns `STALE`, and that callback must not
schedule another task.
- If the stored index is less than `expectedIndex`, the state machine
returns a consistency error, and the callback must not schedule another task.
For `BLOCKED`:
```text
expectedIndex = n
targetIndex = n + 1
```
For `DISCARD`, the callback time is captured exactly once after successful
execution of occurrence `n`:
```text
callbackTime = captured once
lowerBound(t) = min { k >= 0 | executionTime(k) >= t }
expectedIndex = n
targetIndex = max(n + 1, lowerBound(callbackTime))
```
This definition covers callback time exactly equal to an occurrence, delayed
callbacks, multiple missed occurrences, clock rollback, and stale callbacks
from a previous leader.
A callback may schedule the target occurrence only after its exact CAS
returns `ADVANCED`. It must not increment an in-memory occurrence index or
schedule another task after a failed progress write, token mismatch, `STALE`
result, or consistency error.
If a consensus write fails or its result is ambiguous, the callback retries
the same transition using the same `cqToken`, `expectedIndex`, and
`targetIndex`. It does not calculate a different target and does not rebase the
transition on local state.
If the first write committed but its acknowledgement was lost, the retry
observes that the stored index is already greater than `expectedIndex`,
receives `STALE`, and stops without creating a competing schedule chain.
Leader recovery and scheduler reconciliation always read the persisted
`(cqToken, nextOccurrenceIndex)` and install at most one task for that
persisted pair. A stale callback never derives a new chain from its local
execution time.
The recovery flow is:
```text
persisted boundary + persisted durations
+ persisted zone + persisted nextOccurrenceIndex
-> calculate exact occurrence
-> install one task identified by (cqToken, occurrenceIndex)
```
Execution remains at-least-once. If query execution succeeds but the CAS is
not durably committed before a failure, recovery may execute the same
occurrence again. This prevents competing schedule chains but does not
introduce an exactly-once query execution protocol.
Tests will cover old- and new-leader callbacks racing on the same index,
duplicated callbacks, lost or ambiguous consensus responses, token mismatch
after DROP and re-CREATE with the same CQ ID, CAS retry with the same expected
index, `DISCARD` with multiple missed occurrences, equality and clock rollback
cases, and recovery without an off-by-one shift.
### 4. Define a deterministic and implementable duration partial order
After normalization, a user-provided duration is represented as:
```text
D = (M, F)
```
where `M >= 0` is the calendar-month component, `F >= 0` is the fixed-tick
component, and at least one component must be positive when the clause requires
a positive duration.
Duration-to-duration validation uses component-wise dominance:
```text
D1 >= D2 iff M1 >= M2 and F1 >= F2
D1 > D2 iff D1 >= D2 and D1 != D2
```
Examples:
```text
1mo3d > 1mo // accepted
2mo >= 1mo // accepted
12mo == 1y // normalized equality
1mo vs 30d // incomparable
30d vs 1mo // incomparable
2mo vs 1mo40d // incomparable
```
When a required ordering is incomparable, the statement is rejected with a
clear semantic error. This partial order is used for duration-to-duration
constraints such as:
```text
startOffset > endOffset
startOffset >= EVERY
```
No month-to-day approximation is used for these comparisons.
Validation against the fixed configuration value
`continuous_query_minimum_every_interval` uses a separate deterministic
elapsed-time lower bound.
For an EVERY duration `(M, F)`:
```text
if M == 0:
elapsedLowerBound = F
else:
elapsedLowerBound = M * 28d - 36h + F
```
The calculation uses checked arithmetic in the configured timestamp
precision. The bound is based on these platform-level limits:
- Advancing by `M` positive Gregorian calendar months spans at least `M *
28d` in local date/time.
- Java `ZoneOffset` is bounded between `-18:00` and `+18:00`.
- Therefore, the offset difference between the two resolved endpoints can
reduce elapsed time by at most 36 hours.
- The fixed component is applied afterwards as elapsed ticks.
The CQ is accepted only when:
```text
elapsedLowerBound >= continuous_query_minimum_every_interval
```
Otherwise it is conservatively rejected because the minimum interval cannot
be proven.
This does not reuse 28 days per month as if it were an elapsed duration. The
additional 36-hour offset bound accounts for DST and larger historical offset
transitions representable by the Java time model. CREATE acceptance is
therefore independent of the host default time zone and of
TZDB-version-specific transition data. Overflow while calculating the bound is
a semantic error.
Tests will cover equal duration vectors, strict and non-strict
component-wise dominance, incomparable calendar-versus-fixed durations,
compound durations, minimum-EVERY values around the lower-bound threshold, and
checked overflow under `ms`, `us`, and `ns` precision.
### 5. Keep CQ aligned with the existing Tree SQL duration aliases
CQ will not introduce private `month` or `year` aliases in this issue.
The supported calendar units are limited to the existing Tree SQL
abbreviations:
```text
mo
y
```
Unit matching remains case-insensitive, and `y` is normalized to twelve
calendar months.
The following aliases are outside the scope of this issue:
```text
month
months
year
years
```
If full unit aliases are added later, they should be introduced through a
separate Tree SQL grammar change covering all duration positions consistently,
including `GROUP BY TIME`, date arithmetic, `FILL`, `SESSION`, and CQ
`EVERY`/`RANGE`.
Therefore, a CQ will consistently use:
```sql
RESAMPLE EVERY 1mo RANGE 1mo
...
GROUP BY(1mo)
```
This replaces the previous addendum section that proposed CQ-only `month`
and `year` aliases.
### Retained implementation decisions
The following parts of the previous proposal remain unchanged:
- Durations use a structured `(monthPart, nonMonthDuration)` representation.
- `1y` is normalized to `12mo`.
- Every occurrence is derived from the original boundary:
```text
executionTime(n) = calendarApply(B, n * E, Z)
```
- Occurrences are not generated by repeatedly adding a duration to the
previously clamped occurrence.
- RANGE endpoints are derived from the original boundary in duration-vector
space:
```text
startTime(n) = calendarApply(B, n * E - startOffset, Z)
endTime(n) = calendarApply(B, n * E - endOffset, Z)
```
- The boundary, `boundaryExplicit`, zone, normalized durations, CQ token,
and occurrence index are persisted.
- Existing persisted CQs retain their legacy fixed-duration behavior.
- Existing flattened `mo`/`y` CQs are not silently migrated; recreating a CQ
is the explicit opt-in to calendar semantics.
- DataNodes receive concrete query start and end timestamps.
- `TExecuteCQ.timeout` is calculated from adjacent actual occurrence
instants:
```text
deltaTicks = executionTime(n + 1) - executionTime(n)
timeoutMs = ceil(deltaTicks / ticksPerMillisecond)
```
- Restart, leader recovery, and snapshot recovery resume from the persisted
boundary-relative occurrence index.
- All duration multiplication, vector arithmetic, timestamp conversion, and
timeout conversion use checked arithmetic.
### Updated focused test plan
The implementation tests will include:
1. `EVERY 1mo`, `EVERY 1y`, and inherited `GROUP BY(1mo)`/`GROUP BY(1y)`.
2. Rejection of `month`, `months`, `year`, and `years`.
3. CQ and `GROUP BY TIME` instant equality across ordinary dates, month
ends, leap days, DST gaps, and DST overlaps.
4. Version-1 structured request round trips.
5. Rejection of partial, conflicting, unknown-version, and markerless new
requests.
6. Mixed-version calendar CREATE rejection at both DataNode ingress and
ConfigNode acceptance.
7. Required legacy-field sentinel behavior and the hard ConfigNode reader
barrier.
8. Legacy fixed-duration request, plan, procedure, and snapshot recovery.
9. Component-wise duration comparison and incomparable-duration rejection.
10. Fixed minimum-EVERY lower-bound validation.
11. CAS advancement, duplicated callbacks, leader changes, token fencing,
and ambiguous consensus results.
12. `BLOCKED` and `DISCARD` recovery, including equality and clock rollback.
13. Boundary-relative compound schedules and RANGE calculations.
14. Checked overflow and `ms`/`us`/`ns` timeout conversion.
With these changes, DST alignment, mixed-version ingress protection, exact
progress transitions, deterministic comparison rules, and Tree SQL alias
consistency are implementation requirements.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
