You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bifromq.apache.org by "popduke (via GitHub)" <gi...@apache.org> on 2026/10/29 01:34:31 UTC

[PR] Fix dist route ordering and Fact pruning [bifromq]

popduke opened a new pull request, #323:
URL: https://github.com/apache/bifromq/pull/323

   - Aligns trie traversal, Fact updates, and range pruning with storage key order (fixes #332).
   - Encodes empty filter levels as `0xFF` to fix matching for topic filters with empty levels.


-- 
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: commits-unsubscribe@bifromq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix dist route ordering and Fact pruning [bifromq]

Posted by "Gujiawei-Edinburgh (via GitHub)" <gi...@apache.org>.
Gujiawei-Edinburgh commented on code in PR #323:
URL: https://github.com/apache/bifromq/pull/323#discussion_r4408205780


##########
bifromq-dist/bifromq-dist-worker-schema/src/main/java/org/apache/bifromq/dist/worker/schema/KVSchemaUtil.java:
##########
@@ -96,7 +97,8 @@ public static ByteString tenantBeginKey(String tenantId) {
     public static ByteString tenantRouteStartKey(String tenantId, List<String> filterLevels) {
         ByteString key = tenantBeginKey(tenantId);
         for (String filterLevel : filterLevels) {
-            key = key.concat(copyFromUtf8(filterLevel)).concat(SEPARATOR_BYTE);
+            key = key.concat(filterLevel.isEmpty() ? EMPTY_LEVEL_BYTE : copyFromUtf8(filterLevel))

Review Comment:
   Will `EMPTY_LEVEL_BYTE` introduce compatibility issues? For example, unsubscribe an old format topic filter.



-- 
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: commits-unsubscribe@bifromq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix dist route ordering and Fact pruning [bifromq]

Posted by "popduke (via GitHub)" <gi...@apache.org>.
popduke merged PR #323:
URL: https://github.com/apache/bifromq/pull/323


-- 
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: commits-unsubscribe@bifromq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix dist route ordering and Fact pruning [bifromq]

Posted by "Gujiawei-Edinburgh (via GitHub)" <gi...@apache.org>.
Gujiawei-Edinburgh commented on code in PR #323:
URL: https://github.com/apache/bifromq/pull/323#discussion_r4409202931


##########
bifromq-dist/bifromq-dist-worker-schema/src/main/java/org/apache/bifromq/dist/worker/schema/KVSchemaUtil.java:
##########
@@ -96,7 +97,8 @@ public static ByteString tenantBeginKey(String tenantId) {
     public static ByteString tenantRouteStartKey(String tenantId, List<String> filterLevels) {
         ByteString key = tenantBeginKey(tenantId);
         for (String filterLevel : filterLevels) {
-            key = key.concat(copyFromUtf8(filterLevel)).concat(SEPARATOR_BYTE);
+            key = key.concat(filterLevel.isEmpty() ? EMPTY_LEVEL_BYTE : copyFromUtf8(filterLevel))

Review Comment:
   Agreed. I think we can also point it out in this commit message



-- 
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: commits-unsubscribe@bifromq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix dist route ordering and Fact pruning [bifromq]

Posted by "popduke (via GitHub)" <gi...@apache.org>.
popduke commented on code in PR #323:
URL: https://github.com/apache/bifromq/pull/323#discussion_r4409152511


##########
bifromq-dist/bifromq-dist-worker-schema/src/main/java/org/apache/bifromq/dist/worker/schema/KVSchemaUtil.java:
##########
@@ -96,7 +97,8 @@ public static ByteString tenantBeginKey(String tenantId) {
     public static ByteString tenantRouteStartKey(String tenantId, List<String> filterLevels) {
         ByteString key = tenantBeginKey(tenantId);
         for (String filterLevel : filterLevels) {
-            key = key.concat(copyFromUtf8(filterLevel)).concat(SEPARATOR_BYTE);
+            key = key.concat(filterLevel.isEmpty() ? EMPTY_LEVEL_BYTE : copyFromUtf8(filterLevel))

Review Comment:
   Theoretically, yes. MQTT permits empty levels, though they are an unusual choice for topic filters. Such filters may already fail to match correctly with the old encoding.
   If any such subscriptions exist, clients should explicitly unsubscribe from those filters before the upgrade. This requirement should be documented in the release notes.



-- 
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: commits-unsubscribe@bifromq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org