Re: [EXTERNAL] - Re: PR reviewed

[Alex Deparvu]

Hi,

Sorry but I find your request very odd. what would her machine have that
the PR doesn't contain, and why would any of this impact the crave builds?

If you want to take a look, the PR in question is
https://github.com/apache/solr/pull/1632
The branch that originates the PR is
https://github.com/igiguere/solr/tree/SOLR-14886-Suppress_stack_trace_in_Query_response
And I would assume the key to this mystery lies in how far it drifted from
main without being rebased: This branch is 12 commits ahead, 6 commits
behind apache:main.

I have not followed the crave builds discussions very closely, but if I may
offer some debugging idea: if applying the
'magic_build_formula(main_branch, PR_branch)' is failing and you'd like a
reproduce, I would try to: fork Solr main branch with the history intact ->
which gives you main_branch_local, fork this PR's repo (or just pickup the
specific branch with its history intact) which gives you PR_branch_local.
then debug 'magic_build_formula(main_branch_local, PR_branch_local)'. same
formula should result in the same failures I assume. this is all open
source in the end, all data should be available to you already.

Otherwise if you look at older PRs (month+) you'll find other examples
pretty quickly, here is one I'm working on that seem to have the same
troubles https://github.com/apache/solr/pull/1119

I hope this helps,
alex



On Wed, Aug 30, 2023 at 4:50 PM Yuvraaj Kelkar  wrote:

> Hi Isabelle,
>
> I'm from the Crave team who's trying to figure out the PR patching problem.
> I'd like to be able to debug your PR.
>
> I will need to understand what your patch looks like on your machine.
> Will you be willing to join a Zoom call where I guide you to run a few git
> commands?
> This will really help me figure out whats going on, and then recommend
> steps to move your PR forward.
>
> Thanks,
> -Uv
>
> On Aug 25 2023, at 10:00 am, Isabelle Giguere
>  wrote:
> > Hi again;
> >
> > My PR build keeps failing because of unrelated patches being applied.
> > The PR branch has no conflicts with master, and the fork is up-to-date
> (as of writing this, could change any minute), so I don't understand why
> these patches are being applied.
> > Note that I will not have time to look into this in the next few weeks.
> > If anyone with some knowledge of SOLR-16861 and/or SOLR-16809 wants to
> look into this, feel free to use the fork.
> > Thank you
> > --
> > Applying: SOLR-16861: Correct(override) MDC info forerror: patch failed:
> solr/core/src/java/org/apache/solr/core/NodeConfig.java:145
> > error: solr/core/src/java/org/apache/solr/core/NodeConfig.java: patch
> does not apply
> > hint: Use 'git am --show-current-patch=diff' to see the failed patch
> > `CoordinatorHttpSolrCall` (#1753)
> > Applying: SOLR-15734: Add "Major Changes" note for 9.3 v2 API changes
> > Applying: SOLR-16809: Converge logic for hidden sysProps
> >
> > Patch failed at 0115 SOLR-16809: Converge logic for hidden sysProps
> > ...
> > ---
> >
> > Isabelle Giguère
> >
> > 
> > De : Isabelle Giguere 
> > Envoyé : 23 août 2023 15:13
> > À : Mike Drob 
> > Cc : dev solr 
> > Objet : Re: [EXTERNAL] - Re: PR reviewed
> >
> > CAUTION: This email originated from outside of the organization. Do not
> click links or open attachments unless you recognize the sender and know
> the content is safe. If you feel that the email is suspicious, please
> report it using PhishAlarm.
> >
> > Hi Mike;
> > Thanks for the heads-up.
> > Task "validateSourcePatterns" fails if there's no logger specific to the
> test class!
> > I'll fix that... But... Should "validateSourcePatterns" be so picky
> about logging in test classes?
> > Just a thought.
> >
> > Isabelle Giguère
> >
> > 
> > De : Mike Drob 
> > Envoyé : 23 août 2023 12:14
> > À : Isabelle Giguere 
> > Cc : dev solr 
> > Objet : [EXTERNAL] - Re: PR reviewed
> >
> > CAUTION: This email originated from outside of the organization. Do not
> click links or open attachments unless you recognize the sender and know
> the content is safe. If you feel that the email is suspicious, please
> report it using PhishAlarm.
> > Resending with corrected email address
> > On Wed, Aug 23, 2023 at 11:13 AM Mike Drob  md...@mdrob.com>> wrote:
> > Hi Isabelle,
> >
> > It looks like there is a precommit check issue with the code still, can
> you take a look and address that? You should be able to click on "details"
> for the failing check, or run it locally using ./gradlew check -x test
> > Thanks for your contribution!
> > On Wed, Aug 23, 2023 at 9:52 AM Isabelle Giguere
>  wrote:
> > Hello Solr committers!
> >
> > This PR was reviewed by Christine Poerschke<
> https://urldefense.com/v3/__https://github.com/cpoerschke__;!!Obbck6kTJA!duKnEIiSaXVHBEWj835rekhNLQQNmKDttzEvh0as8goQtakJlinf3RMOV1dceYMQVHMyxeHHQXhYjZ0AqmqjPupy72w$
> <
> 

Re: [EXTERNAL] - Re: PR reviewed

[Yuvraaj Kelkar]

Hi Isabelle,

I'm from the Crave team who's trying to figure out the PR patching problem.
I'd like to be able to debug your PR.

I will need to understand what your patch looks like on your machine.
Will you be willing to join a Zoom call where I guide you to run a few git 
commands?
This will really help me figure out whats going on, and then recommend steps to 
move your PR forward.

Thanks,
-Uv

On Aug 25 2023, at 10:00 am, Isabelle Giguere  
wrote:
> Hi again;
>
> My PR build keeps failing because of unrelated patches being applied.
> The PR branch has no conflicts with master, and the fork is up-to-date (as of 
> writing this, could change any minute), so I don't understand why these 
> patches are being applied.
> Note that I will not have time to look into this in the next few weeks.
> If anyone with some knowledge of SOLR-16861 and/or SOLR-16809 wants to look 
> into this, feel free to use the fork.
> Thank you
> --
> Applying: SOLR-16861: Correct(override) MDC info forerror: patch failed: 
> solr/core/src/java/org/apache/solr/core/NodeConfig.java:145
> error: solr/core/src/java/org/apache/solr/core/NodeConfig.java: patch does 
> not apply
> hint: Use 'git am --show-current-patch=diff' to see the failed patch
> `CoordinatorHttpSolrCall` (#1753)
> Applying: SOLR-15734: Add "Major Changes" note for 9.3 v2 API changes
> Applying: SOLR-16809: Converge logic for hidden sysProps
>
> Patch failed at 0115 SOLR-16809: Converge logic for hidden sysProps
> ...
> ---
>
> Isabelle Giguère
>
> 
> De : Isabelle Giguere 
> Envoyé : 23 août 2023 15:13
> À : Mike Drob 
> Cc : dev solr 
> Objet : Re: [EXTERNAL] - Re: PR reviewed
>
> CAUTION: This email originated from outside of the organization. Do not click 
> links or open attachments unless you recognize the sender and know the 
> content is safe. If you feel that the email is suspicious, please report it 
> using PhishAlarm.
>
> Hi Mike;
> Thanks for the heads-up.
> Task "validateSourcePatterns" fails if there's no logger specific to the test 
> class!
> I'll fix that... But... Should "validateSourcePatterns" be so picky about 
> logging in test classes?
> Just a thought.
>
> Isabelle Giguère
>
> 
> De : Mike Drob 
> Envoyé : 23 août 2023 12:14
> À : Isabelle Giguere 
> Cc : dev solr 
> Objet : [EXTERNAL] - Re: PR reviewed
>
> CAUTION: This email originated from outside of the organization. Do not click 
> links or open attachments unless you recognize the sender and know the 
> content is safe. If you feel that the email is suspicious, please report it 
> using PhishAlarm.
> Resending with corrected email address
> On Wed, Aug 23, 2023 at 11:13 AM Mike Drob 
> mailto:md...@mdrob.com>> wrote:
> Hi Isabelle,
>
> It looks like there is a precommit check issue with the code still, can you 
> take a look and address that? You should be able to click on "details" for 
> the failing check, or run it locally using ./gradlew check -x test
> Thanks for your contribution!
> On Wed, Aug 23, 2023 at 9:52 AM Isabelle Giguere 
>  wrote:
> Hello Solr committers!
>
> This PR was reviewed by Christine 
> Poerschke  
> >
>  and Alex 
> D  
> >.
>  Thank you both 
> IMHO, the code is as clean as it's going to get. I hope it can be merged some 
> day.
> https://urldefense.com/v3/__https://github.com/apache/solr/pull/1632__;!!Obbck6kTJA!duKnEIiSaXVHBEWj835rekhNLQQNmKDttzEvh0as8goQtakJlinf3RMOV1dceYMQVHMyxeHHQXhYjZ0AqmqjSTGD-Bw$
>  
> 
> Regards;
> Isabelle Giguère

Re: Circuit breakers and update requests

[Jan Høydahl]

Hi

For those interested in this topic, here is the JIRA, which has a Pull Request 
for review: https://issues.apache.org/jira/browse/SOLR-16954

Jan Høydahl

> 3. jul. 2023 kl. 22:28 skrev Pierre Salagnac :
> 
> Hi Jan,
> 
> As far as I know, Solr only supports circuit breaks for queries at the
> moment.
> 
> We have a custom integration of circuit breakers for indexing (in Solr 8,
> so that's not fully aligned with what in solr 9) with a custom
> UpdateRequestProcessor. Basically, a new instance of every update processor
> is created by calling the corresponding factory. We then check system stats
> (CPU...) at the first handled doc.
> Maybe there are other ways of integrating circuit breakers, but at least
> doing so in an update processor was not intrusive in the code.
> 
> Probably it is worth it to have something standard in Solr. A "symetric"
> class of SearchHandler to add CB integration could be UpdateRequestHandler.
> 
> Pierre
> 
> 
> Le mar. 27 juin 2023 à 13:32, Jan Høydahl  a écrit :
> 
>> Hi,
>> 
>> Solr has CPU and Memory circuit breakers that will terminate Search
>> Requests only.
>> See
>> https://solr.apache.org/guide/solr/latest/deployment-guide/circuit-breakers.html
>> for docs.
>> 
>> Example:
>> 
>>  
>>true
>>75
>>true
>>75
>>  
>> 
>> 
>> A Solr node typically gets overloaded by the combined update and query
>> traffic, and
>> I'm looking into enabling ciricuit breakers for update requests. For many
>> workloads, pausing
>> update traffic would resolve the situation, with the benefit of users not
>> being affected by aborted
>> queries.
>> 
>> So ideally I'd want to be able to choose to enable/disable CB on
>> update/query individually.
>> Or better, to kill update requests on e.g. 80% threshold and search
>> requests on 90% threshold.
>> 
>> The current breaker impl for search <
>> https://github.com/apache/solr/blob/branch_9_2/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java#L372:380>
>> [1] is hardcoded into SearchHandler in such a way that if ANY
>> of the configured breakers trips, search requests are aborted. Also the
>> breakers seem to be generic
>> in nature, named CPUCircuitBreaker <
>> https://github.com/apache/solr/blob/branch_9_2/solr/core/src/java/org/apache/solr/util/circuitbreaker/CPUCircuitBreaker.java>
>> and MemoryCircuitBreaker <
>> https://github.com/apache/solr/blob/branch_9_2/solr/core/src/java/org/apache/solr/util/circuitbreaker/MemoryCircuitBreaker.java>,
>> so adding new UpdateCPUCircuitBreaker
>> does not seem to be the intention here.
>> 
>> I'm also unclear on the usefulness of having CB on the core level and not
>> the node level. If you have
>> 10 cores from 10 collections on a node, and only some have CBs while
>> others do not, the node will
>> still be overloaded unless the admin convinces every collection owner to
>> implement the same CBs?
>> 
>> So my question becomes - how do we enable CB for update requests into this
>> mix in a clean way?
>> 
>> Jan

SOLR-16955 as 9.4 release blocker

[Alex Deparvu]

Hi,

I wanted to let everyone know I have marked SOLR-16955 [0] as a blocker for
9.4 release.

I believe that with tracing enabled performing changes via security apis
will not work (and also Admin UI Security page will not allow for any
changes).
I am evaluating a fix, but until then please let me know if you think this
should not be a blocker.

best,
alex

[0] https://issues.apache.org/jira/browse/SOLR-16955