platinumhamburg commented on PR #1702:
URL: https://github.com/apache/fluss/pull/1702#issuecomment-3303319226
@polyzos Hi, Giannis, I have thoroughly read through this PR, and I feel
that this PR is advancing some excellent work. However, I have some thoughts
about its implementation, and perhaps we can have some discussion about it.
**1. Regarding Client-Side Interface Design**
Considering that the semantics of Lookuper is to provide point queries, I
think defining scan queries in LookUper might not be a good idea. Perhaps
adding the new interfaces to Scan would have more consistent semantics.
Given that: FullScan == Scan without limit
Therefore, a more natural client-side interface semantics would be:
`
table.newScan()
.createBatchScanner(...)
`
or
`
table.newScan()
.createLogScanner(...)
`
If we consider partition pruning requirements, we can define a filter
interface for the Scan type:
`
table.newScan()
.filter(${partition_filter}) // based on Predicate system, we can
temporarily constrain Predicate columns to only include partition columns,
which can avoid committing to row-level filtering in this PR
.createBatchScanner(...)
`
We further consider how the client side can refine the interface
implementation, which involves two types: **TableScan** and **BatchScanner**.
TableScan determines the Scanner type to create based on construction
parameters.
The current BatchScanner implementation is KvSnapshotBatchScanner, which
doesn't meet our needs. We can create a new type that inherits the BatchScanner
interface called DefaultBatchScanner (or RemoteBatchScanner), and the scan
logic newly added in PrimaryKeyLookuper in the current PR can be migrated into
it.
2. Regarding ProtoBuf Protocol Definition
Perhaps we need to consider more points in protocol design:
- Maybe we should explicitly specify the Bucket list. Having TabletServer's
ReplicaManager provide fuzzy filtering may not provide clear consistency
semantics (considering scenarios like upgrades/downgrades, scaling, Coordinator
primary-secondary switching, etc., the distribution state of Replicas may not
be globally consistent)
- For a table with a large number of rows, the behavior of scanning all data
in a single operation is terrible (we cannot restrict which tables users can
perform fullScan on), so we must consider cases where multiple fetches are
needed to complete a full scan. And once interfaces are made public, they must
be stable, so even if we don't actually implement multiple scans in this PR, we
can still define future-oriented protocols at the interface level. From this
perspective, we might need a CreateSnapshot interface to define a temporary
resource in a transient state on the server side and return a SnapshotId;
thereafter we can use this SnapshotId multiple times to execute Scan requests
to scan partial data; finally destroy the snapshot through a CloseSnapshot.
I look forward to hearing your thoughts, and welcome discussion at any time.
--
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]
