goutamadwant opened a new pull request, #12850:
URL: https://github.com/apache/seatunnel/pull/12850
### Purpose of this pull request
Fixes #12849.
Task acknowledgements for a checkpoint run on concurrent Hazelcast operation
threads, and `PendingCheckpoint.acknowledgeTask` was not synchronized. One ack
could remove the last pending task and complete the checkpoint while another
ack had already removed its task but not yet reported its state.
`CompletedCheckpoint` shares the action state map, so the checkpoint could be
stored without that subtask's state. It was still reported as COMPLETED, and a
restore from it started that subtask with empty state.
This PR:
- Runs the pending-set removal, state reporting, statistics update and
fully-acknowledged check under the pending checkpoint's lock. The future is
completed after the lock is released.
- Refuses to store a checkpoint when a task that acknowledged it has no
state in it, and fails it with `CHECKPOINT_INSIDE_ERROR` instead.
- Counts the reported state size in bytes; it was counting state arrays.
- Fixes the acknowledged-subtask count. Statistics are kept per subtask with
one slot each, but were indexed by the subtask index, so every subtask with
index >= 1 was dropped.
#11559 also edits `acknowledgeTask`. It fixes the byte count but not the
ordering, so whichever lands second has a small conflict there.
**Before:** two concurrent acks could produce a stored checkpoint with a
`null` subtask state. The monitor showed fewer acknowledged subtasks than the
total, and a state size far below the real one.
**After:** a checkpoint completes only after every acknowledged state is
recorded. The monitor counts every subtask and reports the state size in bytes.
### Does this PR introduce _any_ user-facing change?
Bug fix only. The checkpoint monitor (REST/CLI) now reports correct
acknowledged counts and state size for actions with parallelism > 1. No config,
API or checkpoint format change.
### How was this patch tested?
- `PendingCheckpointTest` (new):
- a deterministic interleaving of two acks;
- 20 000 rounds of two concurrent acks;
- a statistics test (2 subtasks × 100 bytes → 2 acknowledged, 200 bytes).
- All three fail without the fix. The concurrent test found 16 (JDK 11)
and 134 (JDK 17) checkpoints completed with a missing subtask state.
- `CheckpointCoordinatorTest`: two new tests on a parallelism-2 plan, both
failing without the fix:
- the monitor's acknowledged/total counts and state size, through the
coordinator;
- a checkpoint missing an acknowledged subtask's state is rejected and not
stored.
- `PendingCheckpointTest` and `CheckpointCoordinatorTest`: 21/21 pass on JDK
11 and 17.
- `seatunnel-engine-server` unit tests: 586 run, 0 failures, 5 skipped (same
as `dev`), on JDK 11 and 17.
- E2E not run locally.
### Check list
* [ ] If any new Jar binary package adding in your PR, please add License
Notice according
[New License
Guide](https://github.com/apache/seatunnel/blob/dev/docs/en/developer/new-license.md)
* [ ] If necessary, please update the documentation to describe the new
feature. https://github.com/apache/seatunnel/tree/dev/docs
* [ ] If necessary, please update `incompatible-changes.md` to describe the
incompatibility caused by this PR.
* [ ] If you are contributing the connector code, please check that the
following files are updated:
1. Update
[plugin-mapping.properties](https://github.com/apache/seatunnel/blob/dev/plugin-mapping.properties)
and add new connector information in it
2. Update the pom file of
[seatunnel-dist](https://github.com/apache/seatunnel/blob/dev/seatunnel-dist/pom.xml)
3. Add ci label in
[label-scope-conf](https://github.com/apache/seatunnel/blob/dev/.github/workflows/labeler/label-scope-conf.yml)
4. Add e2e testcase in
[seatunnel-e2e](https://github.com/apache/seatunnel/tree/dev/seatunnel-e2e/seatunnel-connector-v2-e2e/)
5. Update connector
[plugin_config](https://github.com/apache/seatunnel/blob/dev/config/plugin_config)
--
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]
