[
https://issues.apache.org/jira/browse/KUDU-3810?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18124205#comment-18124205
]
ASF subversion and git services commented on KUDU-3810:
-------------------------------------------------------
Commit fc1035f7a725a5cc915d7a4c764fee1c03f2cb8e in kudu's branch
refs/heads/master from Abhishek Chennaka
[ https://gitbox.apache.org/repos/asf?p=kudu.git;h=fc1035f7a ]
[Java] KUDU-3800: Add diff-scan option to return unobservable rows
The Java client side follow up work mirroring the C++ change. Adds
AsyncKuduScanner.DiffScanRowVisibility (OBSERVABLE_ONLY,
INCLUDE_UNOBSERVABLE) and a three argument diffScan() that takes it.
Unlike the C++ side the prior two argument diffScan() is kept, now
delegating with OBSERVABLE_ONLY. It cannot be dropped here: the
released flink-connector-kudu jar that kudu-replication binds to calls
it from KuduSplitGenerator, and that bytecode is never recompiled
against this tree, so removing the overload breaks continuous
replication at runtime with NoSuchMethodError.
When INCLUDE_UNOBSERVABLE is set the scanner sends the visibility on
NewScanRequestPB and advertises the DIFF_SCAN_ROW_VISIBILITY required
feature, so a server predating the feature rejects the scan rather
than silently dropping rows. Scan tokens record the dependency via
ScanTokenPB.Feature.RowVisibility, though feature flags in scan tokens
aren't verified as expected: see KUDU-3810, to be addressed in a
separate changelist.
KuduScanTokenBuilder.build() serializes straight from the builder
fields without running the AsyncKuduScanner constructor, so added the
checks done in the constructor here as well.
OBSERVABLE_ONLY is the wire default, so an OBSERVABLE_ONLY diff scan
emits the same requests and tokens as before this change.
Adds tests for the scanner and scan-token paths. UPSERT lifecycles
inside the diff window, wire-format assertions on the serialized
token, and a required-feature test pinning that OBSERVABLE_ONLY
advertises nothing new. The argument validation tests run against both
newScannerBuilder() and newScanTokenBuilder().
Change-Id: I9d61b2e942c9b2de1b46007f40311d1fb373104a
Reviewed-on: http://gerrit.cloudera.org:8080/24725
Reviewed-by: Alexey Serbin <[email protected]>
Tested-by: Alexey Serbin <[email protected]>
Reviewed-by: Marton Greber <[email protected]>
> ScanTokenPB.feature_flags verification does not reject tokens using unknown
> features
> ------------------------------------------------------------------------------------
>
> Key: KUDU-3810
> URL: https://issues.apache.org/jira/browse/KUDU-3810
> Project: Kudu
> Issue Type: Improvement
> Reporter: Abhishek Chennaka
> Assignee: Abhishek Chennaka
> Priority: Major
>
> ScanTokenPB.feature_flags exists so that a client deserializing a scan token
> produced by a newer client can detect that the token relies on a feature it
> does not implement, and fail cleanly rather than silently running a scan with
> different semantics. Both clients have a check intended to do this, and
> neither check works.
> Java side:
> {{{}KuduScanToken.pbIntoScannerBuilder(){}}}:
> {code:java}
> Preconditions.checkArgument(
> !message.getFeatureFlagsList().contains(ScanTokenPB.Feature.Unknown),
> "Scan token requires an unsupported feature. This Kudu client must be
> updated.");
> {code}
> C++ side:
> {{{}scan_token-internal.cc{}}}:
> {code:cpp}
> for (int32_t feature : message.feature_flags()) {
> if (!ScanTokenPB::Feature_IsValid(feature) || feature ==
> ScanTokenPB::Unknown) {
> return Status::NotSupported(
> "scan token requires features not supported by this client version");
> }
> }
> {code}
> h3. Why neither fires
> {{feature_flags}} is a proto2 {{repeated}} enum, and proto2 enums are closed:
> the generated type is guaranteed to hold only declared values. That guarantee
> is enforced at parse time, so an undeclared value is routed into the
> message's unknown field set (keyed by field number 1) instead of appearing in
> the typed list. Consequences:
> - {{getFeatureFlagsList()}} / {{message.feature_flags()}} return a list that
> looks complete and well-formed, with no indication that an element was elided.
> - {{Feature.Unknown}} is present only if a producer explicitly wrote 0.
> Neither writer does — {{scan_token-internal.cc}} and {{KuduScanToken}} both
> write {{{}Feature::RowVisibility{}}}, the only feature currently defined.
> - {{Feature_IsValid(feature)}} in the C++ loop can never return false,
> because an invalid value never reached {{{}message.feature_flags(){}}}.
> So both checks only ever trip on an explicit zero, which no producer emits.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
