You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2021/01/26 00:41:55 UTC
[GitHub] [lucene-solr] jtibshirani opened a new pull request #2245: Move old field infos format to backwards-codecs.
jtibshirani opened a new pull request #2245:
URL: https://github.com/apache/lucene-solr/pull/2245
We introduced a new `Lucene90FieldInfosFormat`, so the old
`Lucene60FieldInfosFormat` should live in backwards-codecs.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] iverase commented on a change in pull request #2245: LUCENE-9322: Move old field infos format to backwards-codecs.
Posted by GitBox <gi...@apache.org>.
iverase commented on a change in pull request #2245:
URL: https://github.com/apache/lucene-solr/pull/2245#discussion_r564450316
##########
File path: lucene/backward-codecs/src/java/org/apache/lucene/backward_codecs/lucene60/Lucene60FieldInfosFormat.java
##########
@@ -311,6 +312,11 @@ private static IndexOptions getIndexOptions(IndexInput input, byte b) throws IOE
}
}
+ /**
+ * Note: although this format is only used on older versions, we need to keep the write logic
Review comment:
In theory it is possible to try to add a new vector field to one segment create with this format, I guess it would break?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] jtibshirani commented on pull request #2245: Move old field infos format to backwards-codecs.
Posted by GitBox <gi...@apache.org>.
jtibshirani commented on pull request #2245:
URL: https://github.com/apache/lucene-solr/pull/2245#issuecomment-767205967
Thanks @iverase for pointing this out.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] jtibshirani commented on pull request #2245: Move old field infos format to backwards-codecs.
Posted by GitBox <gi...@apache.org>.
jtibshirani commented on pull request #2245:
URL: https://github.com/apache/lucene-solr/pull/2245#issuecomment-767205967
Thanks @iverase for pointing this out.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mikemccand commented on a change in pull request #2245: LUCENE-9322: Move old field infos format to backwards-codecs.
Posted by GitBox <gi...@apache.org>.
mikemccand commented on a change in pull request #2245:
URL: https://github.com/apache/lucene-solr/pull/2245#discussion_r564552680
##########
File path: lucene/backward-codecs/src/java/org/apache/lucene/backward_codecs/lucene60/Lucene60FieldInfosFormat.java
##########
@@ -311,6 +312,11 @@ private static IndexOptions getIndexOptions(IndexInput input, byte b) throws IOE
}
}
+ /**
+ * Note: although this format is only used on older versions, we need to keep the write logic
Review comment:
YW @iverase.
Right, that case should write a whole new segment, using the latest Codec, holding the just indexed document with vector field.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] jtibshirani commented on pull request #2245: LUCENE-9322: Move old field infos format to backwards-codecs.
Posted by GitBox <gi...@apache.org>.
jtibshirani commented on pull request #2245:
URL: https://github.com/apache/lucene-solr/pull/2245#issuecomment-767730507
> I feel like we need some simple "best practices" documentation somewhere on how to bring new Codec/XXXFormats into existence, where/when to move the old ones etc.
This is a good idea, it took some digging to understand what the process/ strategy was. I'll plan to add some docs.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] jtibshirani commented on a change in pull request #2245: Move old field infos format to backwards-codecs.
Posted by GitBox <gi...@apache.org>.
jtibshirani commented on a change in pull request #2245:
URL: https://github.com/apache/lucene-solr/pull/2245#discussion_r564138601
##########
File path: lucene/backward-codecs/src/java/org/apache/lucene/backward_codecs/lucene60/Lucene60FieldInfosFormat.java
##########
@@ -311,6 +312,11 @@ private static IndexOptions getIndexOptions(IndexInput input, byte b) throws IOE
}
}
+ /**
+ * Note: although this format is only used on older versions, we need to keep the write logic
Review comment:
I hope this assumption is accurate, would appreciate someone double-checking it.
##########
File path: lucene/backward-codecs/src/java/org/apache/lucene/backward_codecs/lucene60/Lucene60FieldInfosFormat.java
##########
@@ -311,6 +312,11 @@ private static IndexOptions getIndexOptions(IndexInput input, byte b) throws IOE
}
}
+ /**
+ * Note: although this format is only used on older versions, we need to keep the write logic
Review comment:
I hope this is accurate, would appreciate someone double-checking it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] jtibshirani commented on a change in pull request #2245: LUCENE-9322: Move old field infos format to backwards-codecs.
Posted by GitBox <gi...@apache.org>.
jtibshirani commented on a change in pull request #2245:
URL: https://github.com/apache/lucene-solr/pull/2245#discussion_r564726130
##########
File path: lucene/backward-codecs/src/test/org/apache/lucene/backward_codecs/lucene60/TestLucene60FieldInfosFormat.java
##########
@@ -14,17 +14,15 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-package org.apache.lucene.codecs.lucene50;
Review comment:
This indeed seems like a mistake.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mikemccand commented on a change in pull request #2245: LUCENE-9322: Move old field infos format to backwards-codecs.
Posted by GitBox <gi...@apache.org>.
mikemccand commented on a change in pull request #2245:
URL: https://github.com/apache/lucene-solr/pull/2245#discussion_r564519920
##########
File path: lucene/backward-codecs/src/test/org/apache/lucene/backward_codecs/lucene60/TestLucene60FieldInfosFormat.java
##########
@@ -14,17 +14,15 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-package org.apache.lucene.codecs.lucene50;
Review comment:
Hmm, why did this previously live under `lucene50` sub-package if it was `lucene60` format?
##########
File path: lucene/backward-codecs/src/java/org/apache/lucene/backward_codecs/lucene60/Lucene60FieldInfosFormat.java
##########
@@ -311,6 +312,11 @@ private static IndexOptions getIndexOptions(IndexInput input, byte b) throws IOE
}
}
+ /**
+ * Note: although this format is only used on older versions, we need to keep the write logic
Review comment:
I don't see how a new vector field would ever be written with this format? That should use the latest (90) field infos format.
Rather, I think the case that requires writing is when user opens old index/segment, that had used this `FieldInfosFormat`, and then user updates some doc values in the docs in this segment, and then `IndexWriter` needs to write that new doc values field, along with a new `.fnm` (field infos) file.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] jtibshirani commented on a change in pull request #2245: Move old field infos format to backwards-codecs.
Posted by GitBox <gi...@apache.org>.
jtibshirani commented on a change in pull request #2245:
URL: https://github.com/apache/lucene-solr/pull/2245#discussion_r564138601
##########
File path: lucene/backward-codecs/src/java/org/apache/lucene/backward_codecs/lucene60/Lucene60FieldInfosFormat.java
##########
@@ -311,6 +312,11 @@ private static IndexOptions getIndexOptions(IndexInput input, byte b) throws IOE
}
}
+ /**
+ * Note: although this format is only used on older versions, we need to keep the write logic
Review comment:
I hope this is accurate, would appreciate someone double-checking it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] jtibshirani commented on pull request #2245: LUCENE-9322: Move old field infos format to backwards-codecs.
Posted by GitBox <gi...@apache.org>.
jtibshirani commented on pull request #2245:
URL: https://github.com/apache/lucene-solr/pull/2245#issuecomment-780989835
@mikemccand I opened https://github.com/apache/lucene-solr/pull/2395, it'd be great to get your feedback. I didn't include a section on adding a new format, because this felt like a once-in-a-few-years event? I'm also curious if the repo is the right place for such docs.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] iverase commented on a change in pull request #2245: LUCENE-9322: Move old field infos format to backwards-codecs.
Posted by GitBox <gi...@apache.org>.
iverase commented on a change in pull request #2245:
URL: https://github.com/apache/lucene-solr/pull/2245#discussion_r564545408
##########
File path: lucene/backward-codecs/src/java/org/apache/lucene/backward_codecs/lucene60/Lucene60FieldInfosFormat.java
##########
@@ -311,6 +312,11 @@ private static IndexOptions getIndexOptions(IndexInput input, byte b) throws IOE
}
}
+ /**
+ * Note: although this format is only used on older versions, we need to keep the write logic
Review comment:
Thanks @mikemccand for the clarification.
I was thinking what would happen if a user opens an old index/segment and adds a new document that contains a vector field. You are suggesting that in that case a totally new segment would be created. This method is only called in the case you mention above.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] mikemccand commented on pull request #2245: LUCENE-9322: Move old field infos format to backwards-codecs.
Posted by GitBox <gi...@apache.org>.
mikemccand commented on pull request #2245:
URL: https://github.com/apache/lucene-solr/pull/2245#issuecomment-767910776
> This is a good idea, it took some digging to understand what the process/ strategy was. I'll plan to add some docs.
Thanks @jtibshirani!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] iverase commented on a change in pull request #2245: LUCENE-9322: Move old field infos format to backwards-codecs.
Posted by GitBox <gi...@apache.org>.
iverase commented on a change in pull request #2245:
URL: https://github.com/apache/lucene-solr/pull/2245#discussion_r564545408
##########
File path: lucene/backward-codecs/src/java/org/apache/lucene/backward_codecs/lucene60/Lucene60FieldInfosFormat.java
##########
@@ -311,6 +312,11 @@ private static IndexOptions getIndexOptions(IndexInput input, byte b) throws IOE
}
}
+ /**
+ * Note: although this format is only used on older versions, we need to keep the write logic
Review comment:
Thanks @mikemccand for the clarification.
I was thinking what would happen if a user opens an old index/segment and adds a new document that contains a vector field. You are suggesting that in that case a totally new segment would be created using the latest codec. This method is only called in the case you mention above.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] jtibshirani commented on a change in pull request #2245: Move old field infos format to backwards-codecs.
Posted by GitBox <gi...@apache.org>.
jtibshirani commented on a change in pull request #2245:
URL: https://github.com/apache/lucene-solr/pull/2245#discussion_r564138601
##########
File path: lucene/backward-codecs/src/java/org/apache/lucene/backward_codecs/lucene60/Lucene60FieldInfosFormat.java
##########
@@ -311,6 +312,11 @@ private static IndexOptions getIndexOptions(IndexInput input, byte b) throws IOE
}
}
+ /**
+ * Note: although this format is only used on older versions, we need to keep the write logic
Review comment:
I hope this assumption is accurate, would appreciate someone double-checking it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] jtibshirani merged pull request #2245: LUCENE-9322: Move old field infos format to backwards-codecs.
Posted by GitBox <gi...@apache.org>.
jtibshirani merged pull request #2245:
URL: https://github.com/apache/lucene-solr/pull/2245
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org