cpoerschke merged PR #2030:
URL: https://github.com/apache/solr/pull/2030
--
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-unsubscr...@solr.apache
cpoerschke commented on PR #2030:
URL: https://github.com/apache/solr/pull/2030#issuecomment-1836096447
If there are no further comments or concerns or objections then I'll aim to
merge this PR early next week.
Thanks @holysleeper for this contribution!
--
This is an automated mess
cpoerschke commented on PR #2030:
URL: https://github.com/apache/solr/pull/2030#issuecomment-1819084154
> The `solr:modules:extraction:test` CI failure seems
unrelated/pre-existing/environmental e.g.
https://lists.apache.org/thread/fgmlyf8smtj8qyy14466bqfjhsqhqqjs -- cannot
reproduce local
cpoerschke commented on code in PR #2030:
URL: https://github.com/apache/solr/pull/2030#discussion_r1399162976
##
solr/CHANGES.txt:
##
@@ -96,6 +96,8 @@ Improvements
* SOLR-17041: Make CommitTracker currentTlogSize lazy (Alex Deparvu)
+* SOLR-17050: Use compact JSON for Lea
holysleeper commented on PR #2030:
URL: https://github.com/apache/solr/pull/2030#issuecomment-1819003485
I did an update for a failed tidy check. Also @cpoerschke I did a commit
with your suggestion.
--
This is an automated message from the Apache Git Service.
To respond to the message,
cpoerschke commented on code in PR #2030:
URL: https://github.com/apache/solr/pull/2030#discussion_r1399149645
##
solr/modules/ltr/src/test/org/apache/solr/ltr/store/rest/TestModelManagerPersistence.java:
##
@@ -137,6 +139,24 @@ public void testFeaturePersistence() throws Except
holysleeper commented on code in PR #2030:
URL: https://github.com/apache/solr/pull/2030#discussion_r1398163193
##
solr/CHANGES.txt:
##
@@ -96,6 +96,8 @@ Improvements
* SOLR-17041: Make CommitTracker currentTlogSize lazy (Alex Deparvu)
+* SOLR-17050: Use compact JSON for Le
holysleeper commented on PR #2030:
URL: https://github.com/apache/solr/pull/2030#issuecomment-1817457604
> The contribution looks good and it's interesting. I left some minor
comments that should be addressed in my opinion. Not sure we want also some
integration tests to verify we really st
holysleeper commented on code in PR #2030:
URL: https://github.com/apache/solr/pull/2030#discussion_r1398161886
##
solr/core/src/test/org/apache/solr/util/TestUtils.java:
##
@@ -294,4 +294,23 @@ public void testMergeJson() {
assertEquals(
2L, Utils.getObjectByPath(
alessandrobenedetti commented on code in PR #2030:
URL: https://github.com/apache/solr/pull/2030#discussion_r1397546121
##
solr/core/src/java/org/apache/solr/rest/ManagedResourceStorage.java:
##
@@ -421,8 +421,20 @@ public String getInfo() {
/** Default storage implementation
cpoerschke commented on code in PR #2030:
URL: https://github.com/apache/solr/pull/2030#discussion_r1397523369
##
solr/CHANGES.txt:
##
@@ -96,6 +96,8 @@ Improvements
* SOLR-17041: Make CommitTracker currentTlogSize lazy (Alex Deparvu)
+* SOLR-17050: Use compact JSON for Lea
cpoerschke commented on code in PR #2030:
URL: https://github.com/apache/solr/pull/2030#discussion_r1397519730
##
solr/core/src/java/org/apache/solr/rest/ManagedResourceStorage.java:
##
@@ -421,8 +421,20 @@ public String getInfo() {
/** Default storage implementation that use
alessandrobenedetti commented on code in PR #2030:
URL: https://github.com/apache/solr/pull/2030#discussion_r1394113617
##
solr/core/src/test/org/apache/solr/util/TestUtils.java:
##
@@ -294,4 +294,23 @@ public void testMergeJson() {
assertEquals(
2L, Utils.getObjec
cpoerschke commented on PR #2030:
URL: https://github.com/apache/solr/pull/2030#issuecomment-1808110865
The `solr:modules:extraction:test` CI failure seems
unrelated/pre-existing/environmental e.g.
https://lists.apache.org/thread/fgmlyf8smtj8qyy14466bqfjhsqhqqjs -- cannot
reproduce locally
holysleeper commented on PR #2030:
URL: https://github.com/apache/solr/pull/2030#issuecomment-1806789831
Hello @cpoerschke. I've done the same thing for feature store
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
epugh commented on PR #2030:
URL: https://github.com/apache/solr/pull/2030#issuecomment-1791121376
we really don't want our users actually poking around in ZooKeeper...It
really should be mediated by APIs and treated as an internal "implementation
details"!
--
This is an automate
cpoerschke commented on PR #2030:
URL: https://github.com/apache/solr/pull/2030#issuecomment-1790659304
> ... only the file is stored as compacted ... model-store endpoint will
return a formatted JSON. ...
You're right. I'd assumed users would 'look' directly in ZooKeeper and
forgot
florinbabes commented on PR #2030:
URL: https://github.com/apache/solr/pull/2030#issuecomment-1784171333
> One thing I meant to ask, why not use the PackageStore instead? It also (i
believe!) would replicate a model around the cluster, the same way ZK does...
I am not familiar with Pa
florinbabes commented on PR #2030:
URL: https://github.com/apache/solr/pull/2030#issuecomment-1784170646
> Before proceeding with the code review, let me ask an additional question:
How is the size affecting the current implementation? Zookeeper on
heap/off-heap memory? Zookeeper disk? Apac
florinbabes commented on PR #2030:
URL: https://github.com/apache/solr/pull/2030#issuecomment-1784155535
> Hi Florin!
>
> Thanks for opening this pull request, with a test and scoped to only
change the learning-to-rank model store.
>
> I've added a commit with minor tweaks, fee
epugh commented on PR #2030:
URL: https://github.com/apache/solr/pull/2030#issuecomment-1783338271
One thing I meant to ask, why not use the PackageStore instead? It also (i
believe!) would replicate a model around the cluster, the same way ZK does...
--
This is an automated message fr
epugh commented on PR #2030:
URL: https://github.com/apache/solr/pull/2030#issuecomment-1783337652
I think that if it makes sense to compact the JSON, then just compact it.
It's simple enought to pipe through `jq` or we add formatting options on the
way out.
--
This is an auto
alessandrobenedetti commented on PR #2030:
URL: https://github.com/apache/solr/pull/2030#issuecomment-1781805531
Before proceeding with the code review, let me ask an additional question:
How is the size affecting the current implementation?
Zookeeper on heap/off-heap memory? Zookeeper
cpoerschke commented on PR #2030:
URL: https://github.com/apache/solr/pull/2030#issuecomment-1779183595
Would love to hear perspectives from others here too e.g.
@alessandrobenedetti @epugh @janhoy or @tomglk perhaps? Thank you.
--
This is an automated message from the Apache Git Service.
cpoerschke commented on PR #2030:
URL: https://github.com/apache/solr/pull/2030#issuecomment-1779181373
Hi Florin!
Thanks for opening this pull request, with a test and scoped to only change
the learning-to-rank model store.
I've added a commit with minor tweaks, feel free to r
holysleeper opened a new pull request, #2030:
URL: https://github.com/apache/solr/pull/2030
https://issues.apache.org/jira/browse/SOLR-17050
# Description
Currenly we have a limit for how big a model can be when uploaded to solr.
That is because of zookeeper file size limitatio
26 matches
Mail list logo
