klT45 opened a new pull request, #7971:
URL: https://github.com/apache/shenyu/pull/7971
<!-- Describe your PR here; e.g. Fixes #issueNo -->
Fixes #7717.
### Motivation
`InstanceCheckService` marks an instance as `DELETED` after it stops sending
heartbeats, but the
record itself is kept forever: the in-memory health map never shrinks,
`syncDB()` rewrites every
entry back to `instance_info` on each cycle, and the table keeps growing for
instances that no
longer exist. There is no retention policy separate from the liveness
thresholds (20s offline /
60s delete).
This change adds an opt-in retention policy for expired instance records,
without changing the
default behaviour.
### Changes
- New configuration `shenyu.instance-retention.enabled` (default `false`) and
`shenyu.instance-retention.ttl` (default `7d`), bound by
`InstanceRetentionProperties`.
- `InstanceCheckService.purgeExpiredInstances()` runs right after
`doCheck()` in the existing
liveness scheduler, so it only runs on the node that owns the checks: the
master Admin in
cluster mode (`setup()` is invoked after master election, and `close()`
stops the executor when
mastership is released) or the single standalone Admin.
- Memory: entries that are still `DELETED` and whose last heartbeat is older
than the TTL are
removed from `instanceHealthBeatInfo`, so the map is bounded and no longer
re-synced forever.
The removal re-checks the entry with `computeIfPresent`, so an instance
that heartbeats again
in the same window is kept.
- Database: a new `InstanceInfoMapper.deleteExpired(cutoff)` removes rows
with
`instance_state = 0 AND last_heart_beat_time < cutoff`. The same cutoff
guards the delete, so a
renewed identity is not removed, and legacy rows without a heartbeat are
covered by the same
policy once the feature is enabled.
- Tests:
- cleanup disabled by default;
- only expired `DELETED` entries are removed (fresh `DELETED` and `ONLINE`
entries are kept);
- an instance that heartbeats again before the cleanup cycle is kept;
- legacy records with no heartbeat are expired by the policy;
- `InstanceInfoServiceImpl.deleteExpired` delegates the cutoff to the
mapper.
### Notes
- Opt-in: `enabled: false` by default, so upgrading does not change existing
behaviour.
- The DB sweep is a single `DELETE` per check cycle only when the feature is
enabled; there is no
per-instance round trip. Rows are not archived - archival can be a
follow-up if needed.
- `instance_info.last_heart_beat_time` has no dedicated index today; the
table has one row per
instance, so this is expected to be small. Happy to add an index if
reviewers prefer.
### Test
```
./mvnw -pl shenyu-admin test
-Dtest=InstanceCheckServiceTest,InstanceInfoServiceImplTest,InstanceInfoMapperTest,InstanceInfoMapperOracleSqlTest,InstanceInfoUpsertTest
Tests run: 50, Failures: 0, Errors: 0, Skipped: 0
```
Make sure that:
- [x] You have read the [contribution
guidelines](https://shenyu.apache.org/community/contributor-guide).
- [x] You submit test cases (unit or integration tests) that back your
changes.
- [ ] Your local test passed `./mvnw clean install
-Dmaven.javadoc.skip=true`.
The targeted admin test suite above passes; checkstyle (validate phase)
passes as part of the build.
--
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]
