You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2021/10/05 17:07:51 UTC

[GitHub] [solr] cpoerschke opened a new pull request #329: SOLR-15676: UpdateLog.RecentUpdates.getDeleteByQuery to not return duplicate versions

cpoerschke opened a new pull request #329:
URL: https://github.com/apache/solr/pull/329


   https://issues.apache.org/jira/browse/SOLR-15676
   
   (Keeping this pull request as draft whilst it includes the #328 changes.)


-- 
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: issues-unsubscribe@solr.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] cpoerschke commented on a change in pull request #329: SOLR-15676: UpdateLog.RecentUpdates.getDeleteByQuery to not return duplicate versions

Posted by GitBox <gi...@apache.org>.
cpoerschke commented on a change in pull request #329:
URL: https://github.com/apache/solr/pull/329#discussion_r735751990



##########
File path: solr/core/src/java/org/apache/solr/handler/component/RealTimeGetComponent.java
##########
@@ -1265,10 +1265,12 @@ public void processGetUpdates(ResponseBuilder rb) throws IOException
 
     // TODO: get this from cache instead of rebuilding?
     try (UpdateLog.RecentUpdates recentUpdates = ulog.getRecentUpdates()) {
+      Set<Long> updateVersions = new HashSet<>();

Review comment:
       "should be small" is fairly subjective, how about initial capacity-ing it instead to communicate order-of-magnitude?
   
   ```suggestion
         Set<Long> updateVersions = new HashSet<>(versions.size());
   ```




-- 
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: issues-unsubscribe@solr.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] dsmiley commented on a change in pull request #329: SOLR-15676: UpdateLog.RecentUpdates.getDeleteByQuery to not return duplicate versions

Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #329:
URL: https://github.com/apache/solr/pull/329#discussion_r736022743



##########
File path: solr/core/src/java/org/apache/solr/handler/component/RealTimeGetComponent.java
##########
@@ -1265,10 +1265,12 @@ public void processGetUpdates(ResponseBuilder rb) throws IOException
 
     // TODO: get this from cache instead of rebuilding?
     try (UpdateLog.RecentUpdates recentUpdates = ulog.getRecentUpdates()) {
+      Set<Long> updateVersions = new HashSet<>();

Review comment:
       Sounds good.  We may be oversizing it but ok; it'll be reasonably sized given we loop over `versions`.  We still could use LongSet; why not, right?




-- 
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: issues-unsubscribe@solr.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] cpoerschke merged pull request #329: SOLR-15676: UpdateLog.RecentUpdates.getDeleteByQuery to not return duplicate versions

Posted by GitBox <gi...@apache.org>.
cpoerschke merged pull request #329:
URL: https://github.com/apache/solr/pull/329


   


-- 
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: issues-unsubscribe@solr.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] cpoerschke commented on a change in pull request #329: SOLR-15676: UpdateLog.RecentUpdates.getDeleteByQuery to not return duplicate versions

Posted by GitBox <gi...@apache.org>.
cpoerschke commented on a change in pull request #329:
URL: https://github.com/apache/solr/pull/329#discussion_r736454613



##########
File path: solr/core/src/java/org/apache/solr/handler/component/RealTimeGetComponent.java
##########
@@ -1265,10 +1265,12 @@ public void processGetUpdates(ResponseBuilder rb) throws IOException
 
     // TODO: get this from cache instead of rebuilding?
     try (UpdateLog.RecentUpdates recentUpdates = ulog.getRecentUpdates()) {
+      Set<Long> updateVersions = new HashSet<>();

Review comment:
       `LongSet.add` currently does not return a `boolean` but that could be added -- #370 -- and then yes we could use `LongSet` here.




-- 
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: issues-unsubscribe@solr.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] dsmiley commented on a change in pull request #329: SOLR-15676: UpdateLog.RecentUpdates.getDeleteByQuery to not return duplicate versions

Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #329:
URL: https://github.com/apache/solr/pull/329#discussion_r735199467



##########
File path: solr/core/src/java/org/apache/solr/handler/component/RealTimeGetComponent.java
##########
@@ -1265,10 +1265,12 @@ public void processGetUpdates(ResponseBuilder rb) throws IOException
 
     // TODO: get this from cache instead of rebuilding?
     try (UpdateLog.RecentUpdates recentUpdates = ulog.getRecentUpdates()) {
+      Set<Long> updateVersions = new HashSet<>();

Review comment:
       I was going to suggest `org.apache.solr.util.LongSet` but I suppose we can assume this set won't accumulate more than a handful at most so whatever.  A comment "should be small" or something would be helpful.




-- 
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: issues-unsubscribe@solr.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org