Skip to content
This repository was archived by the owner on Dec 10, 2025. It is now read-only.

Conversation

Copy link
Contributor

Copilot AI commented Dec 3, 2025

Clients were rejecting all delta packets after batch sync because lastChangeId was set to Long.MAX_VALUE, causing legitimate incremental updates (changeId 123, 124, etc.) to fail the staleness check.

Changes

  • SyncSetImpl: Exposed currentChangeId property to read the internal changeCounter
  • ClientboundBatchSyncSetPacket: Changed from Pair<String, Set> to Triple<String, Set, Long> to include each set's current change ID in batch sync
  • Client SyncRegistryImpl: Use actual change ID from batch instead of Long.MAX_VALUE

Before:

fun applyBatchSyncSets(bulk: List<Pair<String, Set<Any?>>>) {
    bulk.forEach { (id, snapshot) ->
        set?.addAllInternal(snapshot)
        lastChangeIds[id] = Long.MAX_VALUE  // All subsequent deltas rejected
    }
}

After:

fun applyBatchSyncSets(bulk: List<Triple<String, Set<Any?>, Long>>) {
    bulk.forEach { (id, snapshot, changeId) ->
        set?.addAllInternal(snapshot)
        lastChangeIds[id] = changeId  // Track actual server state
    }
}

Breaking Change

Protocol modification requires coordinated deployment of clients and servers.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • repo.slne.dev
    • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang.invoke=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-opens=java.base/java.nio.charset=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED --add-opens=java.xml/javax.xml.namespace=ALL-UNNAMED -Xmx4G -Dfile.encoding=UTF-8 -Duser.country -Duser.language=en -Duser.variant -cp /home/REDACTED/.gradle/wrapper/dists/gradle-8.14.3-bin/cv11ve7ro1n3o1j4so8xd9n66/gradle-8.14.3/lib/gradle-daemon-main-8.14.3.jar (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Original prompt

[17:59:45 INFO] [dev.slne.surf.cloud.core.client.sync.SyncRegistryImpl]: Ignoring stale SyncSetDeltaPacket for set tab:entries with changeId 123, last known changeId is 9223372036854775807
[17:59:45 INFO] [dev.slne.surf.cloud.core.client.sync.SyncRegistryImpl]: Ignoring stale SyncSetDeltaPacket for set tab:entries with changeId 124, last known changeId is 9223372036854775807
[17:59:45 INFO] [dev.slne.surf.cloud.core.client.sync.SyncRegistryImpl]: Ignoring stale SyncSetDeltaPacket for set tab:entries with changeId 125, last known changeId is 9223372036854775807
[17:59:45 INFO] [dev.slne.surf.cloud.core.client.sync.SyncRegistryImpl]: Ignoring stale SyncSetDeltaPacket for set tab:entries with changeId 126, last known changeId is 9223372036854775807
[17:59:58 INFO] [dev.slne.surf.cloud.core.client.sync.SyncRegistryImpl]: Ignoring stale SyncSetDeltaPacket for set tab:entries with changeId 127, last known changeId is 9223372036854775807
[17:59:58 INFO] [dev.slne.surf.cloud.core.client.sync.SyncRegistryImpl]: Ignoring stale SyncSetDeltaPacket for set tab:entries with changeId 129, last known changeId is 9223372036854775807
[17:59:58 INFO] [dev.slne.surf.cloud.core.client.sync.SyncRegistryImpl]: Ignoring stale SyncSetDeltaPacket for set tab:entries with changeId 130, last known changeId is 9223372036854775807
[17:59:58 INFO] [dev.slne.surf.cloud.core.client.sync.SyncRegistryImpl]: Ignoring stale SyncSetDeltaPacket for set tab:entries with changeId 128, last known changeId is 9223372036854775807

fix this


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 2 commits December 3, 2025 17:06
Co-authored-by: TheBjoRedCraft <143264463+TheBjoRedCraft@users.noreply.github.com>
Co-authored-by: TheBjoRedCraft <143264463+TheBjoRedCraft@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix stale SyncSetDeltaPacket handling for tab entries Fix stale SyncSetDeltaPacket rejection by tracking actual change IDs Dec 3, 2025
Copilot AI requested a review from TheBjoRedCraft December 3, 2025 17:12
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a critical bug where clients were rejecting all legitimate delta packets after receiving a batch sync. The root cause was using Long.MAX_VALUE as the change ID marker after batch synchronization, which made all subsequent incremental updates (with normal sequential change IDs like 123, 124, etc.) appear "stale" and get rejected.

Key Changes:

  • Exposed currentChangeId property in SyncSetImpl to access the internal change counter
  • Modified ClientboundBatchSyncSetPacket protocol from Pair<String, Set> to Triple<String, Set, Long> to include each set's actual change ID
  • Updated client-side SyncRegistryImpl to track actual server change IDs instead of using Long.MAX_VALUE

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
surf-cloud-core/surf-cloud-core-common/src/main/kotlin/dev/slne/surf/cloud/core/common/sync/SyncSetImpl.kt Added currentChangeId property to expose the internal changeCounter value for protocol serialization
surf-cloud-core/surf-cloud-core-common/src/main/kotlin/dev/slne/surf/cloud/core/common/netty/network/protocol/synchronizing/ClientboundBatchSyncSetPacket.kt Modified packet structure to include change IDs in batch sync data; updated serialization/deserialization to write and read the Long changeId after each set's data
surf-cloud-core/surf-cloud-core-client/src/main/kotlin/dev/slne/surf/cloud/core/client/sync/SyncRegistryImpl.kt Changed applyBatchSyncSets to accept Triple with changeId and use the actual server change ID for staleness tracking

@github-actions
Copy link

github-actions bot commented Dec 3, 2025

Qodana for JVM

542 new problems were found

Inspection name Severity Problems
Unused symbol 🔶 Warning 426
Unused import directive 🔶 Warning 28
Unresolved reference in KDoc 🔶 Warning 27
Redundant nullable return type 🔶 Warning 6
Leaking 'this' in constructor 🔶 Warning 4
Redundant 'suspend' modifier 🔶 Warning 4
Constant conditions 🔶 Warning 3
Control flow with empty body 🔶 Warning 2
Possibly blocking call in non-blocking context 🔶 Warning 1
Incorrect formatting ◽️ Notice 22
Function or property has platform type ◽️ Notice 7
Leaking 'this' in constructor ◽️ Notice 5
@Deprecated annotation without 'replaceWith' argument ◽️ Notice 4
Condition of 'if' expression is constant ◽️ Notice 1
Convert 'object' to 'data object' ◽️ Notice 1
Redundant empty initializer block ◽️ Notice 1

☁️ View the detailed Qodana report

Contact Qodana team

Contact us at qodana-support@jetbrains.com

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

2 participants