[#4598] Supports traffic warm-up when the service is just started #4599
Conversation
| service: | ||
| warmup: | ||
| enabled: false |
There was a problem hiding this comment.
This feature will cause demo-edge fail? Is it an incompatible feature?
There was a problem hiding this comment.
compatible for edge
| String registerTimeStr = StringUtils.isBlank(server.getInstance().getTimestamp()) ? "0" : | ||
| server.getInstance().getTimestamp(); |
There was a problem hiding this comment.
Not all Discovery implementation has timestamp and service center is server time not application time.
There was a problem hiding this comment.
change to set register time by instance property
| // add register time for warm up | ||
| propertiesMap.put(InstancePropertiesConst.REGISTER_TIME_KEY, String.valueOf(System.currentTimeMillis())); |
There was a problem hiding this comment.
Distributed time may not always same for different instance, and weighted for instance is not only depended on instance startup time, but time accessed of the consumer.
There was a problem hiding this comment.
add provider invoke time to calculate weight
| private void refreshMapCache() { | ||
| List<String> removeKeys = new ArrayList<>(); | ||
| for (Map.Entry<String, Long> entry : instanceInvokeTime.entrySet()) { | ||
| if (System.currentTimeMillis() - entry.getValue() > ignoreWarmUpTime) { | ||
| removeKeys.add(entry.getKey()); | ||
| } | ||
| } | ||
| if (CollectionUtils.isEmpty(removeKeys)) { | ||
| return; | ||
| } | ||
| removeKeys.forEach(instanceInvokeTime::remove); | ||
| } |
There was a problem hiding this comment.
Is it removing not exist instance? Seems remove invoked instance will cause problem.
| public class InstancePropertiesConst { | ||
| public static final String REGISTER_TIME_KEY = "registerTime"; | ||
| } |
There was a problem hiding this comment.
If can make it simple, do not depends on register time? I think first access time is quite enough.
| .setNameFormat("warm-up-cache-refresh-%d") | ||
| .build()); | ||
| ignoreWarmUpTime = getIgnoreWarmUpTime(); | ||
| executor.scheduleAtFixedRate(this::refreshMapCache, 0, 10, TimeUnit.MINUTES); |
There was a problem hiding this comment.
Maybe do not need a timer. You can remove not exist instance id every more 10 minutes when choosing server(condition e.g. not exist and last accessed time is more than 30 minutes).
No description provided.