Replace the local HTTP server

http server is not needed now for EPUB publications.

Thanks for review.
some reasons about the issues:

HtmlInjector is now in streamer module, and not visible to navigator module.
HtmlInjector needs field userPropertiesPath and customResources which is also not available in class R2EpubPageFragment or EpubNavigatorFragment.
when range filed of ResourceInputStream object is null,ResourceInputStream will auto detect the length for the resource. wether browser read a byte or ByteArray one time is determined by the browser.
WebViewAssetLoader just process one path:assets, and it use URLConnection.guessContentTypeFromName to guess mime type, so js and font files will got null mime type.

Thanks for the additional details,
I'm working on a new Presentation API which will render userPropertiesPath obsolete. It will probably be merged early next year, I think we can revisit your PR then to use this new API instead which will remove any dependency on the streamer.

OK


WriteBatcherTest has two test asserts when running on three node cluster env

So we can address your issue, please include the following:
Version of MarkLogic Java Client API
develop branch
Version of MarkLogic Server
10.0 nightly build for 10.0-4 release.
Java version
Java 9
OS and version
Linux 3 node cluster env.
Input: Some code to illustrate the problem, preferably in a state that can be independently reproduced on our end
Execute WriteBatcherTest test on develop branch with a nightly ML Server build (in 10.0-4 release period) on 3 node cluster.
Actual output: What did you observe? What errors did you see? Can you attach the logs? (Java logs, MarkLogic logs)

com.marklogic.client.test.datamovement.WriteBatcherTest.testBatchesThreads

Error Message
java.lang.AssertionError: The success listener ran wrong number of times expected:<200> but was:<196>
Stacktrace
java.lang.AssertionError: The success listener ran wrong number of times expected:<200> but was:<196>
	at org.junit.Assert.fail(Assert.java:88)
	at org.junit.Assert.failNotEquals(Assert.java:834)
	at org.junit.Assert.assertEquals(Assert.java:645)
	at com.marklogic.client.test.datamovement.WriteBatcherTest.runWriteTest(WriteBatcherTest.java:666)
	at com.marklogic.client.test.datamovement.WriteBatcherTest.testBatchesThreads(WriteBatcherTest.java:522)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:564)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)

com.marklogic.client.test.datamovement.WriteBatcherTest.testEverything

Error Message
java.lang.AssertionError: The success listener ran wrong number of times expected:<200> but was:<184>
Stacktrace
java.lang.AssertionError: The success listener ran wrong number of times expected:<200> but was:<184>
	at org.junit.Assert.fail(Assert.java:88)
	at org.junit.Assert.failNotEquals(Assert.java:834)
	at org.junit.Assert.assertEquals(Assert.java:645)
	at com.marklogic.client.test.datamovement.WriteBatcherTest.runWriteTest(WriteBatcherTest.java:666)
	at com.marklogic.client.test.datamovement.WriteBatcherTest.testEverything(WriteBatcherTest.java:527)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:

Expected output: What specifically did you expect to happen?
Tests run fine. No assert failures.
Alternatives: What else have you tried, actual/expected?
N/A

Just FYI. This test ran fine on single node env.

I'm not able to reproduce the issue on the cluster with 3 nodes and 9 forests that you set up.

Going to close this issue. Tests on Jenkins env does not fail with 03162020 ML Server build.


Indicate when a request has no body in WASM

C++ filters pass the "end-of-stream" flag to the HTTP header callbacks, and set it to true when a request or response has no body. However, we don't pass this flag via the WASM interface.
Without it, there's no way to know if there is going to be a body, so a filter that is supposed to parse or transform the body may never sees its body callbacks called and has no reliable way to know that this will happen.
If we're talking about changing the ABI anyway, then we should add the end_of_stream flag here too.
Otherwise, we can add some new ABI functions so that a filter can find out if it was already set.

@PiotrSikora : I saw some mention in your last PR about a "new ABI revision." If something like that is coming, would it make sense to try and address this issue first by adding the "end_of_stream" flag to the HTTP header callbacks?

Good point, I'm not sure how we've missed that!
Will add it in the next iteration of the ABI. Thanks!


Expand error checking and unit tests of reading test case data.

Prepare transition from parsedData to metadata proto step 1.

Pull Request Test Coverage Report for Build<PHONE_NUMBER>

23 of 30 (76.67%)  changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.2%) to 56.695%

Changes Missing Coverage
Covered Lines
Changed/Added Lines
%

tools/addrundata/case.go
20
27
74.07%

Totals

Change from base Build<PHONE_NUMBER>:
0.2%

Covered Lines:
1211

Relevant Lines:
2136

💛  - Coveralls


upgrade(installer): Add more telemetry for injector installation

What does this PR do?

Adds a bunch of telemetry spans for injector installation, namely

Socket paths in dedicated span
Docker previous default runtime
Docker previous count of runtimes
Which files are mutated
Which units are overridden to get the environment
Spans on almost each operation for more precise error tracking

Adds package name/version in top-level installation span

Example trace
Motivation
More telemetry
Additional Notes
N/A
Possible Drawbacks / Trade-offs
N/A
Describe how to test/QA your changes
N/A

/merge

/merge -c


Reflect Ingress and LB Class to remote clusters

Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes #(issue)
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Please also note any relevant details for your test configuration.

[ ] Test A
[ ] Test B

/test

/test

/hold

/unhold

/test

/test

/rebase test=true

/rebase test=true

/merge


WIP research logic to ensure secrets - single controller

Description
Logic to ensure namespace secrets moved from pod mutator to dynakube controller. Dynakube controller listens to namespace events to make sure ingestendpoint and init secrets are created ASAP in monitored namespace.
The following scenarios do not trigger dynakube reconcile event:

namespace CREATE event w/o matching selector label
namespace UPDATE#1, UPDATE#2, DELETE sequence
namespace UPDATE w/o dynakube/instance label
namespace UPDATE because of controller doing mapping

Approach using delaying event queue seems to be most promising so far:

in case of incoming stream of events - we can limit the number of dynakube reconciliation calls to a specific number

Approach using rate limited queue:

when incoming stream of namespace events starts the dynakube controller is called too often
From DefaultControllerRateLimiter src:

// DefaultControllerRateLimiter is a no-arg constructor for a default rate limiter for a workqueue.  It has
// both overall and per-item rate limiting.  The overall is a token bucket and the per-item is exponential

How can this be tested?
Script to generate big amount of namespace events:
/bin/bash namespace.sh create
/bin/bash namespace.sh delete
/bin/bash namespace.sh verify

In case of my machine ~600 is still OK, 1000 running parallel processes -> other random processes die.
#!/bin/bash
if [ "${1}" == "create" ]; then

  for i in {1..500} ; do
    cat <<EOF | kubectl create -f - &
apiVersion: v1
kind: Namespace
metadata:
  labels:
    monitor: star
  name: sample-${i}
EOF
  done

elif [ "${1}" == "delete" ]; then

  for i in {1..500} ; do
    cat <<EOF | kubectl delete -f - &
apiVersion: v1
kind: Namespace
metadata:
  labels:
    monitor: star
  name: sample-${i}
EOF
  done

elif [ "${1}" == "verify" ]; then

  for i in {1..500} ; do
    /bin/bash ${0} verifybg sample-${i} &
  done

elif [ "${1}" == "verifybg" ]; then
    if ! kubectl -n ${2} get secrets/dynatrace-data-ingest-endpoint secrets/dynatrace-dynakube-config 2>/dev/null 1>&2 ; then
      echo "secret(s) not found in ${2}"
    else
      echo ${2}
    fi
fi

Checklist

[ ] Unit tests have been updated/added
[ ] PR is labeled accordingly with a single label
[ ] I have read and understood the contribution guidelines

the solution was presented to colleagues


Do not allow organisation tagging for specialist-content

It is currently possible to add organisation tags to specialist-publisher owned content. This is a mistake, because specialist-publisher also sends organisations.
If you would tag a document to an organisation here, it would eventually end up in rummager (because rummager reads tags from the content-api). For clarity, organisations should only be set by specialist-publisher.
We will do a follow up to fix any inconsistencies.
Trello: https://trello.com/c/YoeWtswr

👍


Debian/Ubuntu: Java Classpath misses /usr/share/jitsi

In Ubuntu 20.04 LTS, after installing Jitsi Desktop as described here, I try to
$ jitsi

from the Terminal. This gives me
exec /usr/bin/java -cp 'lib/*:config/' --add-opens=java.base/jdk.internal.loader=ALL-UNNAMED --add-opens=java.base/java.lang=ALL-UNNAMED -splash:/usr/share/jitsi/splash.gif net.java.sip.communicator.launcher.Jitsi
Error: Could not find or load main class net.java.sip.communicator.launcher.Jitsi
Caused by: java.lang.ClassNotFoundException: net.java.sip.communicator.launcher.Jitsi

If I start the Java Runtime Environment directly and append /usr/share/jitsito the parameter cp like
$ java -cp '/usr/share/jitsi/lib/*:/usr/share/jitsi/config/' --add-opens=java.base/jdk.internal.loader=ALL-UNNAMED --add-opens=java.base/java.lang=ALL-UNNAMED -splash:/usr/share/jitsi/splash.gif net.java.sip.communicator.launcher.Jitsi

everything works.

Hm, yes. I think I wrote the script to execute within the working directory and not from /usr/bin, which naturally doesn't make sense once installed. The source is here: https://github.com/jitsi/jitsi/blob/master/resources/install/debian/jitsi. Would you mind creating a PR?

Thanks, yes, saw that. However, although I used the branch new-build three years ago already, building Jitsi Desktop has changed since then again. I have to research how it works now. Easy for you, hours, even days for me. Furthermore, I am working at two different areas right now (libjitsi, …) and Jitsi Desktop itself works for me. In other words, right now, I can only report issues in Jitsi Desktop. In other words, when I create a Pull Request, it was tested at least once. I cannot do that right now because I do not have a build system. So, whoever is faster, please, take over this issue.

That file is not compiled, it is used verbatim. Therefore, I can test it right now. Therefore, I created a Pull Request.


WP_Error( 'too-early-for-url-generator' ) maybe is thrown too late

I get this error while generating my view. But if I use ```$brain->get( 'symfony.generator' )->generate( $route_id, $args )``, it works just fine.

Sorry for this very long delay. I had no very defined idea on what to do with this package.
Now I have, and changes will come..
Thanks for reporting, by the way.

Oh, that's great! Looking forward to it.


gadgets: Implement profile block-io as image-based one

Implement the profile block-io gadget as image-based one. Needs #1925, and it's being done in #2425.

Fixed by #3253


Add new XTENSA_ARCH_HAS_HIFIn

WIP.
On-going discussions:

https://github.com/thesofproject/sof/issues/8697
https://github.com/thesofproject/sof/pull/8720
https://github.com/thesofproject/sof/pull/8733

Cadence offers a lot of coprocessor IP, and we don't need to declare all of it.

Good point, I'll remove 1 and 2. I found them in some old Intel toolchains but no point adding them to Zephyr if they are not in use here. They could easily be added later if needed (= never).

... recapitulates the various XCHAL_HAVE_* preprocessor macros from core-isa.h (which I understand isn't quite enough to discriminate at build time given that we may have other concerns like binary compatibility or toolchain choice that affect how we generate code)

To be really honest  I would love a more formal, short-list of high-level requirements in a single place that explain why I'm doing this. I think most of them can be found in the review comments of https://github.com/thesofproject/sof/pull/8720, https://github.com/thesofproject/sof/pull/8682 and a couple others but that's inconvenient and unreliable.
Or maybe @peter-mitsis will provide a different set of high-level requirements for saving HIFI registers that will magically need the same implementation and we can save that documentation effort :-)

That is, if we have CONFIG_XTENSA_ARCH_HAS_HIFI4=y, then XCHAL_HAVE_HIFI4_VFPU must be defined to 1 in the core-isa.h we're getting off the include path. If it isn't, we're either building for the wrong device or the toolchain or HAL is giving us the wrong headers.

Automatically checking at build time that this duplication does not diverge; good idea, thanks! Note one of the important SOF requirement was NOT to #include <core-isa.h> (https://github.com/thesofproject/sof/pull/8682#issuecomment-1877306254, ...) but maybe in Zephyr it's OK.

Thanks for putting this together! Although I am working on saving/restoring the HiFi registers, it has not been 100% clear to me yet which generation of the HiFi coprocessors would need to be supported. This really helps clarify things.
If we can get those compliance checks fixed, I would really like to see this merged so that I can build off of it.

The much smaller approach https://github.com/thesofproject/sof/pull/8787 has just been merged.
Long story short, we're not duplicating every core-isa.h file in Kconfig for now. Maybe later. We would have had to duplicate each core-isa.h  TWICE: once for Zephyr and once for legacy, pre-Zephyr XTOS.
@peter-mitsis I'm closing this PR but I'm not deleting the branch in case you want to re-use some of it for saving and restoring registers.


Fix tf.nn.softmax

Fixes #8667

Can one of the admins verify this patch?

Jenkins, test this please.
@ilya-edrenkin any chance to take a look at the review comments?

Jenkins, test this please.


Digest call returning flag is data actually changed

I am thinking how to lazily update DOM from reactive model instance. Seems the getState() always returns new instance, and digest call returns undefined. Thus I would always need to generate virtual dom or update real DOM, even if there are no changes
Changing getState() to return same instance if there were no data changes probably would be too much, but at least the digest call might return flag if things were updated in the derived properties.

Hi Gleb,
I'm happy to hear you are trying this out.
One approach that comes to mind is to update the DOM or virtual DOM inside a reactive function, like this.
model({
  dom: [function (a, b){
    updateMyDom(a, b);
  }, "a, b"]
});

Although, perhaps I'm not understanding your use case. I did have some ideas on adding the ability to listen for when the state (as returned from getState()) changes. This would be a callback that gets invoked whenever any public properties change (whenever the return value from getState() changes).
Actually, it might be nice to expose the state itself as a special property, that would eliminate the need for the setState and getState functions. Setting state could be model.state(newState), and getting state could be model.state(). Then, listening for changes in the entire state would look like this.
model.state.on(function (newState){ ... });

It could be set up such that if many public properties are changed in a single tick, this callback would only fire once on the next tick.
Would this be useful for you?

I started working on this to see how it would look, check out the state-as-reactive-property branch if you are interested.
It feels like it makes the implementation a bit more complex, but it definitely makes the API nicer, and adds the sweet new feature of being able to listen for changes across the entire set of public properties.

I have changed the public API such that model.state is now a reactive property, which has the on function for listening to changes. Here are some new tests that demonstrate this functionality:
    it("Should listen for changes in state, getting default empty state.", function (done){
      var model = ReactiveModel()
        .addPublicProperty("x", 5)
        .addPublicProperty("y", 10);

      model.state.on(function (newState){
        assert.equal(Object.keys(newState).length, 0);
        done();
      });
    });

    it("Should listen for changes in state, getting state after one change.", function (done){
      var model = ReactiveModel()
        .addPublicProperty("x", 5)
        .addPublicProperty("y", 10);

      model.state.on(function (newState){
        assert.equal(Object.keys(newState).length, 1);
        assert.equal(newState.x, 15);
        done();
      });

      model.x(15);
    });

    it("Should listen for changes in state, getting state after two changes.", function (done){
      var model = ReactiveModel()
        .addPublicProperty("x", 5)
        .addPublicProperty("y", 10);

      model.state.on(function (newState){
        assert.equal(Object.keys(newState).length, 2);
        assert.equal(newState.x, 15);
        assert.equal(newState.y, 45);
        done();
      });

      model.x(15).y(45);
    });

Closing as I think this addresses your original question. Feel free to reopen if I didn't understand your question correctly.


Fixes #4 Improved error messaging for login

Added error message logging for the initial authorization flow.
Example ouput:
[2015-02-28 01:08:06,611] [StorifierBot-18da] Authorization Failed, status: "-100" , message "That's not the right password. Sorry!"
[2015-02-28 00:05:12,611] [StorifierBot-18da] Authorization Failed, status: "403" , message "'The network you are connected to has been temporarily blocked because of suspicious activity"

This will also allow people to better understand failures of their bots, as raised in Issue #4

Good idea, thanks.


Fix RTL connected accounts alignment.

Before:

After:
Chrome:

Firefox:

@jpevarnek do you mind taking a look -- I remember you fighting to align the multi-line userInfo field with the up and down arrow. Does this preserve the original styling?


linter-elm-make freeze when selecting an elm file

Everytime I open Atom and open an elm file the interface entirely freezes.
So after restarting it 20 times I opened the debugger and found the call stack of this every time I paused it, so it seems like a loop:
win32.dirname (path.js:320)
copySync (C:\Users\<username>\…opy-sync.js:22)
(anonymous function) (C:\Users\<username>\…opy-sync.js:40)
copySync (C:\Users\<username>\…opy-sync.js:37)
(anonymous function) (C:\Users\<username>\…opy-sync.js:40)
copySync (C:\Users\<username>\…opy-sync.js:37)
(anonymous function) (C:\Users\<username>\…opy-sync.js:40)
copySync (C:\Users\<username>\…opy-sync.js:37)
(anonymous function) (C:\Users\<username>\…opy-sync.js:40)
copySync (C:\Users\<username>\…opy-sync.js:37)
(anonymous function) (C:\Users\<username>\…opy-sync.js:40)
copySync (C:\Users\<username>\…opy-sync.js:37)
(anonymous function) (C:\Users\<username>\…opy-sync.js:40)
copySync (C:\Users\<username>\…opy-sync.js:37)
copyProjectToWorkDirectory (linter-elm-make.js:445)
lint (linter-elm-make.js:393)
promises.push.Promise.then._this.emitter.emit.linter (linter-registry.js:63)
(anonymous function) (linter-registry.js:62)
lint (linter-registry.js:60)
(anonymous function) (linter.coffee:98)
module.exports.Emitter.simpleDispatch (C:\Users\<username>\…t\lib\emit…:25)
module.exports.Emitter.emit (C:\Users\<username>\…\lib\emit…:125)
lint (editor-linter.js:175)
Commands.lint (commands.coffee:60)
Commands.subscriptions.add.atom.commands.add.linter:lint (commands.coffee:13)
module.exports.CommandRegistry.handleCommandEvent (C:\Users\<username>\…egistry.js:260)
(anonymous function) (C:\Users\<username>\…-registry.js:3)
module.exports.CommandRegistry.dispatch (C:\Users\<username>\…egistry.js:160)
forceLintActiveElmEditor (linter-elm-make.js:854)
(anonymous function) (linter-elm-make.js:169)
module.exports.Emitter.simpleDispatch (C:\Users\<username>\…t\lib\emit…:25)
module.exports.Emitter.emit (C:\Users\<username>\…\lib\emit…:125)
(anonymous function) (C:\Users\<username>\…ntainer.js:346)
module.exports.Emitter.simpleDispatch (C:\Users\<username>\…t\lib\emit…:25)
module.exports.Emitter.emit (C:\Users\<username>\…\lib\emit…:125)
module.exports.Pane.setActiveItem (C:\Users\<username>\…rc\pane.js:282)
module.exports.Pane.activateNextRecentlyUsedItem (C:\Users\<username>\…rc\pane.js:340)
commandRegistry.add.pane:show-next-recently-used-item (C:\Users\<username>\…commands.js:11)
module.exports.CommandRegistry.handleCommandEvent (C:\Users\<username>\…egistry.js:260)
(anonymous function) (C:\Users\<username>\…-registry.js:3)
module.exports.KeymapManager.dispatchCommandEvent (C:\Users\<username>\…ap\lib\ke…:580)
module.exports.KeymapManager.handleKeyboardEvent (C:\Users\<username>\…ap\lib\ke…:388)
module.exports.KeymapManager.terminatePendingState (C:\Users\<username>\…ap\lib\ke…:561)
module.exports.KeymapManager.handleKeyboardEvent (C:\Users\<username>\…ap\lib\ke…:432)
module.exports.WindowEventHandler.handleDocumentKeyEvent (C:\Users\<username>\…-handler.js:98)
(anonymous function) (C:\Users\<username>\…t-handler.js:3)

EDIT:  Ctrl+Enter apparently submits...  >.>
Either way, it seems to be iterating over every path in the project, including node_modules paths, which is sizable because of a certain svg pictograph package, and the program seems to be copying that entire thing out every single time a file is opened, and this takes about ten minutes (after waiting to see if it would complete now that I saw what was happening).  Why is it copying node_modules stuff and how do I get it to stop so it works in this project again (without removing this node_module as it is required for this project)?

Oh and yes, the "Work Directory" option is blank, which based on the text means it should not be copying anything, and yet...

Decided to see what all it was doing while copying files, thus:

It seems to be copying everything to the System temporary directory
The options passed in to copysync are:  options = Object {preserveTimestamps: true, recursive: true, clobber: true, dereference: false}
performCopy seems to remain false, yet the iteration over the fails takes a tremendous amount of time...

Hi!
Thank you for the details.  If Lint On The Fly is enabled, the linter will copy the project files (*.elm, elm-package.json, and elm-stuff) into a temporary directory if Work Directory is not specified.  It will then just watch for changes after that.

The copyProjectToWorkDirectory function has this comment:
    // Recursively copy `.elm` files, `elm-package.json`, and `elm-stuff` from project directory to work directory.
    // Initial linting might take time depending on the project directory size.

A lot of time apparently, and from looking in the source for this function it does seem to only match out files that are in the project root elm-stuff directory, the elm-package.json file in the root project directory, or any files ending with .elm.
However, because of how it is implemented it will iterate over every-single-file testing each of those conditions.  Instead I would recommend parsing the elm-package.json file (it is just json after all) and look in the "source-directories" key for the directories to look in, at that point just copy out "<projectroot>/elm-package.json", "<projectroot>/elm-stuff/", and for each "source-directory" look in "<projectroot>/<source_directory>".
However, why does Lint On The Fly require copying things out to a temporary directory anyway?  If it is just to check for changes then are file-stamps insufficient?

Just copying those in "source-directories" might work :)
Files are copied to a temporary directory so that this can be possible:

In the screenshot, changes in a buffer affect the lints in another even if you haven't saved them yet.
What's your idea with the file-stamps?  Sorry, I'm a little slow right now.  I have a flu and is getting sleepy.

Ah is it because you are having elm-make process all the files instead of stdin'ing them in?  That would probably work better with it.  Ignore the timestamps bit then.  :-)
I know how the flu is, was going around here not long ago.  >.>
But yes, source-directories + elm-package.json + elm-stuff would definitely be best, copying a gig of node_modules every load is... quite painful.  ^.^

AFAIK, there's no stdin option in elm-make as it requires file path(s) as argument(s) and we couldn't just pass the changed files because of issues related to https://github.com/mybuddymichael/linter-elm-make/issues/32.
I was actually waiting for someone like you with a big enough project to test how Lint On The Fly will scale :)  The Work Directory is an optimization you can use so that files will not be copied anymore the next time you restart Atom.
I think the "source-directories" bit is a good idea :+1:

Lol, glad to be of service then.  NPM is always a bane of my existence.  ^.^
And yeah, this is a fairly huge project, Elm integration is rather new in it but growing rapidly (so much better than javascript).

Haha thanks!
I'll get a rest for now (I'm in a different time zone).  I'll check your other messages tomorrow :)  Cheers!

Tested again with no working directory set, no freeze this time.  :-)

Nice!
I hope the release did not break someone's workflow :D


Always dereference the experiment in spawn

What does this PR do?
Attempts to avoid the flaky hangs seen in our CI:
FAILED tests/loggers/test_all.py::test_logger_created_on_rank_zero_only[CSVLogger]
FAILED tests/loggers/test_all.py::test_logger_created_on_rank_zero_only[MLFlow]
FAILED tests/loggers/test_all.py::test_logger_created_on_rank_zero_only[TestTubeLogger]

Closes #10805 (can land first)
Does your PR introduce any breaking changes? If yes, please list them.
None?
Before submitting

[ ] Was this discussed/approved via a GitHub issue? (not for typos and docs)
[x] Did you read the contributor guideline, Pull Request section?
[x] Did you make sure your PR does only one thing, instead of bundling different changes together?
[n/a] Did you make sure to update the documentation with your changes? (if necessary)
[n/a] Did you write any new necessary tests? (not for typos and docs)
[n/a] Did you verify new and existing tests pass locally with your changes?
[x] Did you list all the breaking changes introduced by this pull request?
[n/a] Did you update the CHANGELOG? (not for typos, docs, test updates, or internal minor changes/refactorings)

PR review

[ ] Is this pull request ready for review? (if not, please submit in draft mode)
[x] Check that all items from Before submitting are resolved
[x] Make sure the title is self-explanatory and the description concisely explains the PR
[x] Add labels and milestones (and optionally projects) to the PR so it can be classified

cc @carmocca @akihironitta @borda

This did not work.


CommandHandler Issue

I get the error message:
CommandHandler | No owner channels created! Make sure you've specified correct OwnerId in the credentials.json file.
When running NadekoBot and I do not know what I have specified wrongly~
Blurred out my personal credits only in the screenshot because I suppose this is public.
For ClientId and BotId, I used the APP DETAILS > Client ID: on discordapp.com/developers/applications/me.
For the token I used APP BOT USER > Token: on the same site as above.
For OwnerIds I did @MyDiscordName#MyHashtag and copy pasted only the numbers.
I hope anyone has anyclue whats the issue.

no you need to type \@YourDiscordName and then copy only the numbers you get (its a long number)

I have done that :/

I do not see my mistake in here o:

try removing the trailing comma

dotnet crashes when i do that

11:14:38 BotCredentials | Could not parse the JSON file. Error on line number '8': ']
"LoLApiKey": "",'.
11:14:38 BotCredentials | System.FormatException: Could not parse the JSON file. Error on line number '8': ']
"LoLApiKey": "",'. ---> Newtonsoft.Json.JsonReaderException: After parsing a value an unexpected character was encountered: ". Path 'OwnerIds', line 8, position 2.
at Newtonsoft.Json.JsonTextReader.ParsePostValue()
at Newtonsoft.Json.JsonTextReader.Read()
at Newtonsoft.Json.Linq.JContainer.ReadContentFrom(JsonReader r, JsonLoadSettings settings)
at Newtonsoft.Json.Linq.JContainer.ReadTokenFrom(JsonReader reader, JsonLoadSettings options)
at Newtonsoft.Json.Linq.JObject.Load(JsonReader reader, JsonLoadSettings settings)
at Microsoft.Extensions.Configuration.Json.JsonConfigurationFileParser.Parse(Stream input)
at Microsoft.Extensions.Configuration.Json.JsonConfigurationProvider.Load(Stream stream)
--- End of inner exception stack trace ---
at Microsoft.Extensions.Configuration.FileConfigurationProvider.Load(Boolean reload)
at Microsoft.Extensions.Configuration.ConfigurationRoot..ctor(IList`1 providers)
at Microsoft.Extensions.Configuration.ConfigurationBuilder.Build()
at NadekoBot.Services.Impl.BotCredentials..ctor() in C:\Users\Thorsten\Desktop\Nadeko\NadekoBot\src\NadekoBot\Services\Impl\BotCredentials.cs:line 45

try "OwnerIds": [youridhere],

still the same error

Its a json file, not a regular text file you have removed the spaces before you owener id, use the credentials example.json and copy paste everything.
Remember do not add any extra character or white space.

even with copy pasting the one kwoth send me it does not work

i have checked if its just my discord, but others accounts dont work either

i re-added nadeko to my server and it worked o:

now i just need to get this thing with google to work xD


Steps missing

I thing two steps are missing for the Net Core 3.1:
On Program.cs the options.Connect:
var settings = config.Build();
config.AddAzureAppConfiguration(options =>
{
options.Connect(settings["ConnectionStrings:AppConfig"]);
});
On Startup.cs:
app.UseAzureAppConfiguration();

Document Details
⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

ID: 2984100d-7d78-2115-40f4-4b9478a5a5bc
Version Independent ID: 24ac6adc-6779-b847-ce81-c9d36e4b067b
Content: Quickstart for Azure App Configuration with ASP.NET Core
Content Source: articles/azure-app-configuration/quickstart-aspnet-core-app.md
Service: azure-app-configuration
GitHub Login: @lisaguthrie
Microsoft Alias: lcozzens

@SNorzagaray Thanks for the feedback! We are currently investigating and will update you shortly.

Hi @SNorzagary - thanks for the feedback, and apologies for taking so long to get back to you.
The quickstart just covers the basic scenario of loading app configuration from App Config at startup. The options you showed are required when you want your app to refresh configuration dynamically from App Config, without needing to restart the app. This is covered in the dynamic configuration tutorial.
Let me know if you have any questions on this. Thanks!


Fix symbol conflict with std library

On Ubuntu 18.04, build fails because of this symbol conflict with name "equivalent".
The solution is to rename it with something else.

Closing for obsolescence


Improve linting

Improves linting configs. Runs linting on the repo.
Details:

Prettier now covers every file in the repo
On commit hook now runs both Prettier and ESLint
CI now fails on both Prettier and ESLint errors
ESLint now uses cache - runs faster locally

Note: ESLint currently has a blind spot on .ts files. Fixing that blindspot is out of scope of this PR.

Coverage: 92.392%. Remained the same when pulling c18949613f154a686efae6027aa6a44abe722bbf on ripecosta:rc/linting into 0d27bcb60b9bfd57fabf896715350089e0b1050c on knex:master.

it's failing, workflow yamls shouldn't be modified

it's failing, workflow yamls shouldn't be modified

I think the issue is the use of "[[" in the npm scripts. Fix incoming.


[SamsungMobile] Update relevant end of life dates

Samsung did some changes near November 17th, reflected changes around that.
Devices missing in November 17th I marked as EOL on that date, those there but not there today are marked as EOL on December 31st.

Do we want to add a note to the bottom that says:

Some dates are based on when support status was changed on the Samsung website - they might be approximate.

Or is that kinda implied?

Do we want to add a note to the bottom that says:

Some dates are based on when support status was changed on the Samsung website - they might be approximate.

Or is that kinda implied?

Sounds reasonable, does position in https://github.com/endoflife-date/endoflife.date/pull/826/commits/9ddc4162b200cce94597cb081c62610c73efd400 look ok?

Moved it to the bottom.

Galaxy Tab A 10.5 (2018) is still listed on the website - can you check once?

Galaxy Tab A 10.5 (2018) is still listed on the website - can you check once?

You're right, thanks for catching.


Performance?

At least on my PC (which should be fast enough), time per frame ranges from 2-5ms, but we need <~1.4ms to hit 700Hz

Will add some benchmarks to test frame times

Added perf counters in logs, benchmarks are hard without exposing crate internals


upgraded go-sysinfo

Please label this PR with one of the following labels, depending on the scope of your change:

Enhancement

What does this PR do?
Upgrades the go-sysinfo package

Why is it important?

Checklist

[ ] My code follows the style guidelines of this project
[ ] I have commented my code, particularly in hard-to-understand areas
[ ] I have added tests that prove my fix is effective or that my feature works

Author's Checklist
Related issues

:hourglass_flowing_sand: Build in-progress, with failures

Buildkite Build
Commit: da92a53802ba8d84452fcb41cba41d798fc861c0

Failed CI Steps

:buildkite: Lint

You need to run mage notice to update the notice file

:hourglass_flowing_sand: Build in-progress, with failures

Buildkite Build
Commit: 5d59be78e348540bf7cb688e75bad3523f1c91b3

Failed CI Steps

:buildkite: Lint

History

:broken_heart: Build #231 failed da92a53802ba8d84452fcb41cba41d798fc861c0

:green_heart: Build Succeeded

Buildkite Build
Commit: b2e90db8c145691096ca62c85a74b127aa54c115

History

:broken_heart: Build #235 failed 5d59be78e348540bf7cb688e75bad3523f1c91b3
:broken_heart: Build #231 failed da92a53802ba8d84452fcb41cba41d798fc861c0


lnd chain confusion thinks its on mainnet not testnet

Background
Running bitcoind on testnet and running lnd on testnet with bitcoind, after updating to beta 0.5 several issues are occurring, Automatic RPC configuration fails, (have to specify the bitcoind rpc settings in the lnd.conf. Once started lnd thinks its on mainnet when creating a invoice.
Your environment
lnd version 0.5.0-beta commit=a1f549754b61c1ba84466f0226997687127f7fc0
4.15.0-24-generic #26~16.04.1-Ubuntu
bitcoind "version": 160200
grep "^[^#;]" lnd.conf
[Application Options]
listen=<IP_ADDRESS>:9745
restlisten=<IP_ADDRESS>:8888
externalip=<IP_ADDRESS>:9745
alias=Geektech lnd
color=#6600cc
[Bitcoin]
bitcoin.active=1
bitcoin.testnet=1
bitcoin.node=bitcoind
[Btcd]
[Bitcoind]
bitcoind.rpcuser=thisisatestfool
bitcoind.rpcpass=thisisatestpasword
bitcoind.zmqpubrawblock=tcp://<IP_ADDRESS>:28332
bitcoind.zmqpubrawtx=tcp://<IP_ADDRESS>:28332
[neutrino]
[Litecoin]
[Ltcd]
[autopilot]
[tor]
grep "^[^#;]" bitcoin.conf
testnet=1
server=1
rpcuser=thisisatestfool
rpcpassword=thisisatestpasword
rpcallowip=<IP_ADDRESS>/32
minrelaytxfee=0
txindex=1
daemon=1
zmqpubrawblock=tcp://<IP_ADDRESS>:28332
zmqpubrawtx=tcp://<IP_ADDRESS>:28332
Steps to reproduce
start lnd either using lnd.conf or with cmdline
lnd --bitcoin.active --bitcoin.testnet --debuglevel=trace --bitcoind.rpcuser=thisisatestfool --bitcoind.rpcpass=thisisatestpasword --bitcoind.zmqpubrawblock=tcp://<IP_ADDRESS>:28332 --bitcoind.zmqpubrawtx=tcp://<IP_ADDRESS>:28332
Try and create a invoice.
lncli addinvoice --amt 50000 --memo testing
Expected behaviour
Invoice should be created successfully
Actual behaviour
[lncli] unable to read macaroon path (check the network setting!): open /home/ligntning/.lnd/data/chain/bitcoin/mainnet/admin.macaroon: no such file or directory
lnd thinks its on mainnet but its on testnet the mainnet directory does not exist due to running on testnet.

This was a change in 0.5, but you can set the the --network=testnetflag, or do something like:
alias tncli=lncli --network=testnet

Should have the release notes up by tomorrow to cover some of this stuff.

which section should this go in lnd.conf? [Application Options] or [bitcoin]

is their a change to selecting the bitcoin node also? not thinks im on btcd not bitcoind
Attempting automatic RPC configuration to btcd

lnd.conf is fine, you only need to modify the args to lncli.

Got it thanks


 It's there for a reason.
If you don't give me some info, how could you think that I could solve your problem without it?
Post any error logs you are receiving. Also, if you use HTTP (non-HTTPS) protocol, you need to enable HTTP support (check the README.md to see how to do it).
Use this template to give more info:
## Environment

| Technology           | Version       |
| -------------------- | ------------- |
| Flutter version      |               |
| Plugin version       |               |
| Android version      |               |
| iOS version          |               |
| Xcode version        |               |

Device information: <!-- Manufacturer and model -->

## Description

**Expected behavior:**

**Current behavior:**

## Steps to reproduce

<!-- Optionally provide the least amount of code that shows this behaviour. -->

1. This
2. Than that
3. Then

## Images <!-- if available, else delete -->

## Stacktrace/Logcat <!-- if available, else delete -->

Thanks


Remove elements from cache on DEL events

I don't think that we're currently removing ifaces based on DEL events. In face, I think we're calling e.g. viface_cb with a different etype, but that's it. We need do a purge either in the cfxn callbacks, or assign a different cfxn based on etype. I think the former is better.

This ties in to #28, of course.


more control over faker datatypes and generated mocks

User story.
As a developer, I can pass faker function arguments, so that I can further customize my mocks.
Is your feature request related to a problem?
No. Actually, I would like to thank the team behind prism and ask for feature that gives developers more control over the generated mocks
Describe the solution you'd like
for now, I read the docs and went through the code quickly, I couldn't find a way to customize the faker datatypes, for example, paragraph with 5 sentences or firstName and gender as female.. etc
Additional context
I would suggest having something along the lines of x-faker: name.firstName female

@Nilegfx Thanks for your input! We actually have another issue describing this same feature request:
https://github.com/stoplightio/prism/issues/1647
We'll use that one to track this feature request, and I'll close this one.
Feel free to add any additional info or ideas to that ticket.


[AlwaysOn Profiler] two layers cache - fixes

Why
Fixes to #501
What
Fix issues when the volatile cache or stable cache could remove memory used by another layer cache.
Tests
Change settings to make the increase chance for bug occurence to
constexpr auto kMaxFunctionNameCacheSize = 1000;
constexpr auto kMaxVolatileFunctionNameCacheSize = 5;

Execute
nuke BuildTracerHome BuildAndRunWindowsIntegrationTests  -Framework net6.0 -TargetPlatform x64 --Filter Datadog.Trace.ClrProfiler.IntegrationTests.AlwaysOnProfilerTests

Fail on current main. Passed on branch.
Notes
Kudos to @lachmatt for making really good code review, even when the PR was merge.

Kudos to @lachmatt for making really good code review, even when the PR was merge.

+1!


Sequentially apply loop inversion

Interestingly, this seems to undo some of the smax and select optimizations, as well as a loop strength reduction test
edit: there's also a big bump on CI time, probably because loop inversion enables a bunch of other optimizations (which is not extracted nonetheless)

Probably the big bump in CI time is due to there being two egraph passes instead of one, right? It's the time spent running all the helpers
Hopefully we can reduce with some scheduling

We probably should figure out why the smin optimization isn't firing any more, since there are not any loops in that example right?


"Stream accepts 0 type arguments" error

cache/hosted/pub.dartlang.org/flutter_map-0.2.0/lib/src/map/flutter_map_state.dart:34:3: Error: 'Stream' expects 0 type arguments.
compiler message:   Stream<Null> _merge(LayerOptions options) {

Can you include the code you have to use it.

The problem stems from the fact that flutter_map assumes all streams to be streams of null values. We could relax this by changing from Stream<Null> to Stream<dynamic>?

I think the recommended way is to use void

@johnpryan, I agree, if the stream should not carry any information. However, I could see use cases where the option. stream parameter could be used to notify a plugin that some state has changed, where actual state is consumed by the plugin from the stream directly. This allow for a reactive programming model without any additional custom code for pulling or pushing the state to the plugin.

To fix this and #164 will require some more information and solid steps to reproduce. AFAICT this is not affecting all users right now.

this issue seems stale, so I'm going to close it.


Acceptance Test Provisioner: SSH Client spawns a tty

This is the first in a set of two PRs that'll allow the acceptance tests to run as the default user.  Running acceptance tests without a tty cause flakes on rackspace.  Running the tests as the root user requires us to modify the root account and sshd.  This has slowed down development on GCE.
In the future these changes will be used to allow the provisioning code to be reused to build a better flocker installer.

Thanks @justnoise

I tested the branch with the standalone script that you supplied and saw that the sudo command worked.

Also noted that the script fails when tested against master.

I attempted to run a sudo command using my openssh client and saw sudo: sorry, you must have a tty to run sudo

Then tried again with ssh -t (which seems to be equivalent to the feature in this branch) and the sudo command worked.

I left a few comments above. Feel free to address those in followup branches though, since you've got a rare green build on this PR at the moment.
Merge when you're happy with everything.


Add support for converting to PDF

Would be neat if it was possible to convert to .pdf files instead. Some PDF formats are scalable (vector graphics), so if the .eps was also scalable, that would be great

Thank you that's a great idea! 👏🏼
I think that's totally possible.
I'll be trying to implement the support to converting EPS to PDF files soon! 🚀

Neat!
You might be able to use some code from this vscode extension: https://github.com/ahnafnafee/PostScript-Preview

Support to PDF format added!

Commit: https://github.com/lunatic-fox/eps2img/commit/999128d99019d2e27d8f561da29cf305005be91f

I've just needed to edit a few lines of the original code. 🙂
If everything is ok, I'll be closing this issue.

Neat! You might be able to use some code from this vscode extension: https://github.com/ahnafnafee/PostScript-Preview

Thanks for the suggestion! 😃

PS: I'd totally forgot that .pdf is also a vector format. 😅
Therefore, it's importable to Inkscape, for instance.

All done! 🚀


⚠ Feedaty widget has degraded performance

In 2aaaed5, Feedaty widget (https://widget.feedaty.com/public/js/feedaty.js?merchant=10213975&style_ver=2021&data-sku=36263) experienced degraded performance:

HTTP code: 200
Response time: 3490 ms

Resolved: Feedaty widget performance has improved in 19ad3bb after 21 minutes.


Suricata alerts not showing up in telegraf config

Important notices
Before you add a new report, we ask you kindly to acknowledge the following:
[-] I have read the contributing guide lines at https://github.com/opnsense/plugins/blob/master/CONTRIBUTING.md
[-] I have searched the existing issues and I'm convinced that mine is new.
[-] The title contains the plugin to which this issue belongs
Describe the bug
Intrusion Detection Alerts input not showing in /usr/local/etc/telegraf.conf
To Reproduce
Steps to reproduce the behavior:

Go to Services > Telegraf > Input
Check Intrusion Detection Alerts and save
Restart to Telegraf
ssh into firewall
cat /usr/local/etc/telegraf.conf

Expected behavior
Checking the box places the following into /usr/local/etc/telegraf.conf
[[inputs.tail]]
  data_format = "json"
  files = ["/var/log/suricata/eve.json"]
  name_override = "suricata"
  tag_keys = ["event_type","src_ip","src_port","dest_ip","dest_port"]
  json_string_fields = ["*"]

Environment
OPNsense 21.1.5 (amd64, LibreSSL).
Proxmox VM and libvirt virtual machines
Network Intel

Are you able to get any metrics using this configuration?
According to https://github.com/influxdata/telegraf/tree/master/plugins/inputs/tail

Metrics are produced according to the data_format option. Additionally a tag labeled path is added to the metric containing the filename being tailed.

Nothing is showing here in my InfluxDBv2 installation, even though eve.json contains data :o

OPNsense 21.1.8_1-amd64
InfluxDB 2.0.7
Telegraf 1.19.0


How to store PyAny in hashmap?

🐛 Question
I am trying to create a router that has a route and PyAny as the KV pair in a hashmap.
(PyAny is an async function that is being passed from python to Rust and I want to push it to a threadpool, hence I am not using &PyAny or futures in the initial stage.)
I have two questions for the same:

How to convert the PyAny object to Py
I read on one of the issues in this repo only that I need to store the PyAny object as Py

This is how the hashmap is looking like
        let get_routes: HashMap<Route, Py<PyAny>> = HashMap::new();

and this is how I am appending to the hashmap
    pub fn add_route(&mut self, route: String, handler: PyAny) {
// how do I convert this to Py<PyAny>
        self.get_routes
            .insert(Route::new(RouteType::Route(route)), handler);
    }

When I am implementing the #[pymethods] trait on my implementation, it is giving me this error:

[rustc E0277] [E] the trait bound `pyo3::PyAny: pyo3::FromPyObject<'_>` is not satisfied

expected an implementor of trait `pyo3::FromPyObject<'_>`

note: required because of the requirements on the impl of `pyo3::FromPyObject<'_>` for `pyo3::PyAny`

What does the above error mean and how can I fix it?
This is how the impl looks like at the moment:
#[pymethods]
impl Server {
    // pub fn new(port: usize, number_of_threads: usize) -> Self {
    #[new]
    pub fn new() -> Self {
        let url = format!("<IP_ADDRESS>:{}", 5000);
        let get_routes: HashMap<Route, Py<PyAny>> = HashMap::new();
        Self {
            port: 5000,
            number_of_threads: 1,
            router: Router::new(),
            threadpool: ThreadPool::new(1),
            listener: TcpListener::bind(url).unwrap(),
            get_routes, // not implemented in router as unable to match lifetimes
        }
    }

Thanks in advance 😄

You need to take the argument as Py<PyAny> or &PyAny.  In the latter case you have to convert it into a new Py reference with handler.into().

What does the above error mean and how can I fix it?

This error is an indication the the type (pyo3::PyAny as given in the first line of the error message) is not supported by PyO3 as a function/method argument.
In this case, it is because PyAny is not a useful type: it needs to be behind a GIL reference (&'py PyAny) or in a GIL-independent container (Py<PyAny>).

Hi @birkenfeld . Thank you for the answer.

n the latter case you have to convert it into a new Py reference with handler.into()

How do I do that? Should I use the following syntax  let f = handler.into(Py<PyAny>);?

into() does not take arguments; the type to convert to is a type parameter.
Normally just
let f = handler.into();

will suffice, if type inference can identify what you want to get out.  Otherwise you can use
let f: Py<PyAny> = handler.into();

But in the case of add_route above, you should write it like this:
    pub fn add_route(&mut self, route: String, handler: Py<PyAny>) {
        self.get_routes
            .insert(Route::new(RouteType::Route(route)), handler);
    }

Hi @birkenfeld ,
Thank you for the help! Can you please also tell me how can I convert Py<PyAny> back to &PyAny.
If possible can you point me to the documentation so that I don't nag you everytime 😅
I found some documentation here: https://pyo3.rs/v0.13.2/types.html?highlight=Py#pyt-and-pyobject , but I am unable to figure it out from here.
Thank you again!

as_ref(py) should do it.

@birkenfeld, how do I get the py object (which I need to pass as a param) ?

I think we'd better continue this in https://gitter.im/PyO3/Lobby - issues are not the ideal way for this kind of help.


feat: allow fetching previous versions of meltano state via the CLI

Migrated from GitLab: https://gitlab.com/meltano/meltano/-/issues/3417
Originally created by @tayloramurphy on 2022-04-19 14:46:33

Accepting a --state-id=_ CLI flag in meltano state get to retrieve a prior version of the state (as enumerated via something like meltano state list --history=true).

Still relevant. Updated the title to indicate the actual feature.


feat(Data.Set.Ncard): enat set cardinality

This PR is a second attempt at defining the cardinality of a set as extended natural number, with a function encard. The implementation involves a refactor, where the existingncard function is redefined in terms of encard. This shortens a lot of proofs, reduces reliance on the Finset API, and allows for a potential future refactor where ncard is removed if it is decided to be redundant.

This looks pretty nice thanks!  Thanks for refactoring Set.ncard to use Set.encard.
We may want to rename the file (in a different PR probably) to Data.Set.Card after this is merged.

Thanks for the comments. I'm on board with the rename - I'll PR it once this one is through.

@apnelson1 just FYI, I think you may have missed a bunch of the comments (GitHub automatically folds long sections of the PR/comment history and you have to click to unfold it).

@apnelson1 just FYI, I think you may have missed a bunch of the comments (GitHub automatically folds long sections of the PR/comment history and you have to click to unfold it).

Oops/Thanks! - I think they're all addressed now.

bors d+

bors r+

bors r+


[BUG] Async shard fetches taking up GBs of memory causing ClusterManager JVM to spike with large no. of shards (> 50k)

Describe the bug
GatewayAllocator performs async fetch for unassigned shards to check if any node has shards on its local disk. These async fetches are done per shard. When nodes are dropped in the cluster, it results in unassigned shards and GatewayAllocator performs async fetches when nodes try to join back. In a cluster which has large no. of shards in 10s of thousands it results in too many async fetches causing cluster manager JVM to spike.
In one of the cluster which had > 50K shards, async fetches was taking 13GB memory.

Expected behavior
These async fetches should be batched per node instead of being executed per shard per node.
OpenSearch Version : 1.1
Additional context
Also, these async fetches were discussed earlier in the context of taking up lot of CPU while sending these requests during reroute operation. https://github.com/elastic/elasticsearch/issues/57498 It should also be evaluated if these requests should be sent from different threadpool instead of blocking the masterService#updateTask which is single threaded and needed for processing cluster state updates faster.

I tried doing a repro of the issue, wherein I found that approx 82-83% of heap is being used by GatewayAllocator.
Steps of repro:

Create a cluster of 50 data nodes, and with around 130k shards.
Restarted all 50 nodes in a rolling fashion
Had a cron that took active heap dumps on master during that time.

On analysis found that DiscoveryNode is part of response of AsyncShardFetch and is consuming considerable amount of memory.
Screenshots:

There can be another improvement area around the cache strategy used in AsyncFetch. It occupies significant chunk of master's memory (about 60%) in case of large cluster restarts.
The Cache currently stores two types of data, NodeGatewayStartedShards and NodeStoreFilesMetadata.
We can also think if there is any scope of improvement in these objects store or not.(One is definitely as stated above the DisoveryNode one).
ScreenShot:

Step of repro were same as here: https://github.com/opensearch-project/OpenSearch/issues/5098#issuecomment-1502893705

On analysis found that DiscoveryNode is part of response of AsyncShardFetch and is consuming considerable amount of memory.

Today, considering responses from all the nodes are cached for an unassigned shard so that GatewayAllocator can generate the complete picture. It caches the empty the response as well which can be replaced by a dummy object. This will reduce the DiscoveryNode class overhead significantly from n (nodes) * m (unassignedshards) to m only.

Overall Proposal
Context
During node-left and node-join we do a reroute for assigning unassigned shards. To do this assignment Cluster Manager fetches the shards metadata in async manner on per shard basis. This metadata contains the information about the shard and its allocation. We need this to take a new decision on allocation. reroute is the crucial method call of AllocationService which takes care of this flow for whole cluster.
public ClusterState reroute(ClusterState clusterState, String reason)
Once we receive the data from all the nodes (all nodes relevant for the shardId), we build allocation decision and perform reroute. [I’ll explain code flow in detail upcoming section]
Problem
In case there are 50K/100K shards, during cluster restart scenarios, we end up making transport calls for each and every shard. And all transport threads are doing this same work(same code flow for different shardId) which chokes the transport and it make the cluster unstable. OOM kill is one of the results of this situation as there are back to back calls.
Spike in JVM is seen due to so many objects of DiscoveryNode class, which are coming in response of the transport calls made to the data nodes. As this is a base class in transport call response class, it’s always present for each shard call’s response.
Acronyms :
GA - GatwayAllocator
PSA - PrimaryShardAllocator
RSA - ReplicaShardAllocator
IPSA - InternalPrimaryShardAllocator
Current flow
Let’s understand the code flow a bit better.
Node left scenario
Coordinator.Java has a method removeNode which is the handler for any node failure. It submits a task for node-left with executor type NodeRemovalClusterStateTaskExecutor
https://github.com/opensearch-project/OpenSearch/blob/90678c2d76ed1c31aebc85a23fe5be12f6e81417/server/src/main/java/org/opensearch/cluster/coordination/Coordinator.java#L318-L320
execute method of this particular executor triggers AllocationService
https://github.com/opensearch-project/OpenSearch/blob/90678c2d76ed1c31aebc85a23fe5be12f6e81417/server/src/main/java/org/opensearch/cluster/coordination/NodeRemovalClusterStateTaskExecutor.java#L123
Above method unassign shards that are associated with a node which is not part of the cluster. Executor always pass reroute flag as true.
This method first fail all the shards present on bad nodes, then triggers reroute method of AllocationService.
https://github.com/opensearch-project/OpenSearch/blob/90678c2d76ed1c31aebc85a23fe5be12f6e81417/server/src/main/java/org/opensearch/cluster/routing/allocation/AllocationService.java#L317-L318
reroute calls internal private reroute method which triggers the allocation of existing unassigned shards.
https://github.com/opensearch-project/OpenSearch/blob/90678c2d76ed1c31aebc85a23fe5be12f6e81417/server/src/main/java/org/opensearch/cluster/routing/allocation/AllocationService.java#L539
https://github.com/opensearch-project/OpenSearch/blob/90678c2d76ed1c31aebc85a23fe5be12f6e81417/server/src/main/java/org/opensearch/cluster/routing/allocation/AllocationService.java#L544-L570
Now for each primary and replica shard, allocateUnassigned method of GatewayAllocator class is called which triggers
https://github.com/opensearch-project/OpenSearch/blob/90678c2d76ed1c31aebc85a23fe5be12f6e81417/server/src/main/java/org/opensearch/gateway/GatewayAllocator.java#L168
innerAllocate calls allocateUnassigned method of BaseGatewayShardAllocator which tries to make allocation decision:
https://github.com/opensearch-project/OpenSearch/blob/90678c2d76ed1c31aebc85a23fe5be12f6e81417/server/src/main/java/org/opensearch/gateway/BaseGatewayShardAllocator.java#L76
makeAllocation decision is written separately for primary and replica in PSA & RSA respectively. PSA calls fetchData method of IPSA(written in GA file) to find the metadata about this shard from data nodes using transport call:
https://github.com/opensearch-project/OpenSearch/blob/90678c2d76ed1c31aebc85a23fe5be12f6e81417/server/src/main/java/org/opensearch/gateway/PrimaryShardAllocator.java#L114
GA maintains a map where key is shardId and value is a fetcher object, this fetcher object is used for actually making the transport call and fetch the data.
https://github.com/opensearch-project/OpenSearch/blob/90678c2d76ed1c31aebc85a23fe5be12f6e81417/server/src/main/java/org/opensearch/gateway/GatewayAllocator.java#L284-L297
fetchData method is written in AsyncFetchShard class, as this is an async method, we intelligently triggers reroute in the async response of this method. Once data is returned, reroute will trigger the flow but won’t call data nodes again as it’ll see that required data is already received in the cache. Because we keep looking in the cache and check which are remaining for fetching using following call within fetchData method:
https://github.com/opensearch-project/OpenSearch/blob/90678c2d76ed1c31aebc85a23fe5be12f6e81417/server/src/main/java/org/opensearch/gateway/AsyncShardFetch.java#L139
Based on the result of this data - AsyncShardFetch.FetchResult<TransportNodesListGatewayStartedShards.NodeGatewayStartedShards> shardState
We build the result of which nodes we can allocate this shard to and mark allocation decision as YES.
Based on this BaseGatewayShardAllocator’s allocataeUnassigned method initializes the shard.
https://github.com/opensearch-project/OpenSearch/blob/90678c2d76ed1c31aebc85a23fe5be12f6e81417/server/src/main/java/org/opensearch/gateway/BaseGatewayShardAllocator.java#L83-L91
Node join scenario
JoinTaskExecutor has the code for the actual Task which gets executed on node-join, it triggers a reroute as part of its flow in case cluster manager is changed or a new node joins. In case cluster state is updated with existing nodes, as part of applying the cluster state IndicesClusterStateService triggers shard action - shart/started or shard/failure. Both started and failed executors(ShardStateAction class) have a handler in case cluster state is published from cluster manager and triggers reroute:
https://github.com/opensearch-project/OpenSearch/blob/90678c2d76ed1c31aebc85a23fe5be12f6e81417/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java#L206-L209
Shard Started
https://github.com/opensearch-project/OpenSearch/blob/90678c2d76ed1c31aebc85a23fe5be12f6e81417/server/src/main/java/org/opensearch/cluster/action/shard/ShardStateAction.java#L829-L836
Shard Failed
https://github.com/opensearch-project/OpenSearch/blob/90678c2d76ed1c31aebc85a23fe5be12f6e81417/server/src/main/java/org/opensearch/cluster/action/shard/ShardStateAction.java#L546-L559
So we know ultimately it’ll trigger the whole flow of assigning unassigned shards.
Solution
Key bottleneck we found by profiling is too many calls of the same fetchData transport(too many serialization/deserialization) as well as the in memory load of too many DiscoveryNode objects once response is received and until it is garbage collected. *More analysis of heap dump needs to be done for understanding more data points on memory side. *
Phase 1: Batch calls for multiple shards
First solution is to batch call for multiple shards which is being done 1 per shard today. This will contain different request type as we take multiple data structures similar to what we send today for 1 shard. Response structure will also change accordingly. This batching can be done on node level or with a fix batch count depending on performance or failure scenarios.
A draft PR has been opened already https://github.com/opensearch-project/OpenSearch/pull/7269 , this has rough code for making the call on node level. As we know today 1K shards can be present on a node, we can discuss further if that number is okay or we need configurable batching number to give more resiliency to the solution. Based on this discussion, the solution will change a little bit as currently I'm suggesting changes only for per node level call.
Phase 2 : Reduce Discovery Node objects
We must reduce DiscoveryNodeObjects as they are being created via new when we receive the response as part of code :
public NodeGatewayStartedShards(StreamInput in) throws IOException {
    super(in);
}

Code inside super
protected BaseNodeResponse(StreamInput in) throws IOException {
        super(in);
        **node = new DiscoveryNode(in);**
    }

Part 1:
We reduce the number of these objects based on the number of calls we make - for example we make a batch of 100 and send 1000 calls. So, only 1000 DiscoveryNode objects will be present depending on for which node we’re sending the call. This way we reduce DN objects as well as save on ser/desr work. We also need to keep a version check(>3.0) for destination node so BWC is maintained(as we know older version nodes won't have this handler).
Changes required
We need to change the flow starting from AllocationService to all the way down to the transport call (fetchData).
Surrounding changes :

Need to introduce a new method like allocateUnassignedBatch or allocateUnassignedPerNode in the interface ExistingShardsAllocator , because today it has allocateUnassigned and that is tightly written for per shard.
Use the newly written method in allocateExistingUnassignedShards method based on version to avoid BWC issues.
Construct a map like <Node, List> , iterate this map and call allocateUnassignedPerNode passing one entry of this map.
Write a new implementation of allocateUnassignedPerNode in GatewayAllocator which calls the allocateUnassignedPerNode of BaseGatewayShardsAllocator
Add a new method similar to makeAllocationDecision, as makeAllocationDecision makes sense for allocations done on specific nodes. As the actual implementation of this method is written in PSA/RSA. They need to write a new implementation and return a List [will discuss later if abstracting out the makeAllocationDecision all the way to ExistingShardsAllocator would make sense ??]
BaseGatewayShardsAllocator then use these list of decisions to do the actual allocation.

Core changes required for new transport call :
AsyncShardFetch.FetchResult<NodeStoreFilesMetadata> shardStores = fetchData(unassignedShard, allocation);

This fetch data needs to be changed for a list of shards not just a single shard so a new method is required in both PSA & RSA with new implementation.
As the fetcher objects of GA needs to do the work per node basis the core map of per shard fetcher

private final ConcurrentMap<
        ShardId,
        AsyncShardFetch<TransportNodesListGatewayStartedShards.NodeGatewayStartedShards>> asyncFetchStarted = ConcurrentCollections
            .newConcurrentMap();

will change to a different type like
private final ConcurrentMap<
        NodeId,
        AsyncShardBatchFetch<TransportNodesListGatewayStartedShards**Batch**.NodeGatewayStartedShards**Batch**>> nodeLevelShardFetchers = ConcurrentCollections
            .newConcurrentMap();

Currently request contains shardId and customDataPath, new request will have a List of <ShardId, CustomDataPath> or map of same in case .get method is required in implementation.
Similarly response contain an allocationId in NodeGatewayStartedShards, new response would have the mapping of <shardId, NodeGatewayStartedShards> .
Key thing is that the overall response from 1 node will be having 1 DiscoveryNode object in base class BaseNodeResponse, so for many shards (or all shards on a node) there will be reduced number of DiscoveryNode object in response anyways.
Implementation of the transport call on node will be similar to current nodeOperation code but it'll work of a number of shards not just 1.
Cache object which contains the information about which node's fetching is in progress or done or failed, will be flipped to maintain similar information for shardId (in our node level fetcher class's cache).

Changes for failure handling or correctness

We'll keep getting new set of shards which are unassigned if more nodes go down. For that we need to maintain an atomic set where shards are maintained for which fetch is already happening and async success call will clear it out from the set. This way we always make sure that we can cleanly send a new request for different set of shards if new nodes fail. Before sending a call, we must check and get the diff using that atomic/synchronized set.
In case result was failed for a particular shard (as request is being executed for single node it's hard to see partial failures, but still..), we'll either restart fetching based on retryable exceptions or permanently fail the fetch as it is done today.

https://github.com/opensearch-project/OpenSearch/blob/90678c2d76ed1c31aebc85a23fe5be12f6e81417/server/src/main/java/org/opensearch/gateway/AsyncShardFetch.java#L261-L270
Part 2:
We keep the DiscoveryNode objects limited to the number of nodes exactly. As we know only fixed number of nodes should exist or join/drop the cluster, we keep some common cache for all this particular information and refer it(instead of doing new) whenever needed based on ephemeralId or some other relevant id. This way we can save on memory. As first solution itself is giving some relief in number of DiscoveryNode objects, we can debate more on if this is truly required based on gains diff of part 1 and part 2.
Impact
A benchmarking was run to see that we can scale well with batching approach.
Cluster can scale up to 100-120K shards without facing any OOM kill where current code faces OOM kill with 45K shards. [more details on benchmarking will be attached below]
Note : Current POC code also contain changes where we don’t rely on BaseNodeResponse class so number of DiscoveryNode objects are reduced to number of calls only and right now that is per node, so part2 of the phase 2 solution is automatically covered. But based on discussion on these approaches, I'll write actual code
Pending Items:
Note 2: Integration tests are failing, need to look at those.
AI: Also need to explore if we need to change the GA async fetch call to generic threadpool instead of using clusterManager updateTask

@sachinpkale @Bukhtawar @andrross @shwetathareja looking for feedback on proposal.

@sachinpkale @Bukhtawar @andrross @shwetathareja gentle reminder.

@reta - would love some feedback.

Thanks @amkhar for explaining the problem in detail.
Couple of points on the Phase 1 solution for batching (more around implementation):

The batch size for no. of unassigned shards queried per node can be configurable.
Lets say during first round of reroute, 10 shards were unassigned and a request was sent to all nodes for those 10 unassigned shards, now 1 more shard got unassigned, would it send a new request with that one shard or send with 11 shards or wait for first request to return before sending a new request. would like to understand this more how are you handling this with atomic set as called out above. Would this set be maintain across fetching round?
On a particular node, while fetching a particular shard information, it should have timeout so that if it is unable to collect information for one shard, it shouldn't keep other shards also waiting.
What will be the behavior during mixed mode upgrades would it send batch request to new nodes and per shard to old nodes?
Are there enough integration tests for the current flow, if not please add new tests upfront so that we can establish correctness via those tests after you make the change.
Lets say Leader sent a request r1 to node n1 and assumed it timed out and sent another one r2 as well. But the response for first request r1 arrived later after r2. In this case, old fetch round responses should be discarded. But now consider the case from Point 2 where 2 fetch requests were sent with different shardId list to same node and both responses should be processed. How are both the cases differentiated and handled?

Regarding Phase 2 for reducing no. of discovery node objects:
These async fetches are done per unassigned shard per node as leader doesn't know which node has the shard. But the major overhead comes as it caches the response as is and for cases where shard is not present (which will be the majority) is also stored as is and has DiscoveryNode object. As you are making calls per node and response structure is already changed, then do you still need explicit changes for DiscoveryNode, that should be addressed automatically?
Be careful when changing ExistingShardsAllocator interface to ensure it is not a breaking change as it is exposed via ClusterPlugin
https://github.com/opensearch-project/OpenSearch/blob/9d9a143ff7a896bcfb116fb133f84718b73dec8b/server/src/main/java/org/opensearch/plugins/ClusterPlugin.java#L84
This also means you will need to keep per shard fetching marked as deprecated in 2.x and can be removed in 3.0 only.

In case result was failed for a particular shard (as request is being executed for single node it's hard to see partial failures, but still..), we'll either restart fetching based on retryable exceptions or permanently fail the fetch as it is done today.

will it fetch only for failed shards or all the shards in that batch? What is the current retry mechanism?

Cache object which contains the information about which node's fetching is in progress or done or failed, will be flipped to maintain similar information for shardId (in our node level fetcher class's cache).

Today, the cache can get per shard view easily, Now it has to iterate over all entries in the top level map with NodeId as the key to check the shard status. Right?

Add a new method similar to makeAllocationDecision, as makeAllocationDecision makes sense for allocations done on specific nodes. As the actual implementation of this method is written in PSA/RSA. They need to write a new implementation and return a List [will discuss later if abstracting out the makeAllocationDecision all the way to ExistingShardsAllocator would make sense ??]

How would it impact _cluster/allocation/explain API which can be triggered per shard?

Thanks Aman, per node calls make sense, I am assuming this is only being done for assigning primary shards first and then replica shards subsequently. It would be easier to review the code once you have the draft ready

@reta - would love some feedback.

Thanks @amkhar , the details go way beyond my knowledge but batching per node looks very logical

@shwetathareja

The batch size for no. of unassigned shards queried per node can be configurable.

Yes.

Lets say during first round of reroute, 10 shards were unassigned and a request was sent to all nodes for those 10 unassigned shards, now 1 more shard got unassigned,

We have this cache object today, it'll be utilized to know which shards are already being fetched for a particular node.
private final Map<String, NodeEntry<T>> cache = new HashMap<>();
will be changed to
private final Map<ShardId, ShardEntry<T>> cache = new HashMap<>();
Similarly our current findNodesToFetch method will change to findShardsToFetch, that way we identify from cache, which all shards should be sent for fetching and skip the ones which are already being fetched.

would it send a new request with that one shard

Yes

or send with 11 shards or wait for first request to return before sending a new request.

No and No

On a particular node, while fetching a particular shard information, it should have timeout so that if it is unable to collect information for one shard, it shouldn't keep other shards also waiting.

We've this piece of code per shard which is being executed on the data node.
https://github.com/opensearch-project/OpenSearch/blob/03bc19220a711853660652a4761c05285f597884/server/src/main/java/org/opensearch/gateway/TransportNodesListGatewayStartedShards.java#L153-L218
A for loop is required to run it for multiple shards, I don't think running this in parallel new threads is a good idea. So, yes, we can add a timeout for per shard execution, if any of the shard fetch code execution(one execution of loop code) takes time, we can mark this as failed shard with exception type as OpenSearchTimeOut and return the response accordingly.

Lets say Leader sent a request r1 to node n1 and assumed it timed out and sent another one r2 as well.

Even today, we don't assume any time outs. Listener gets a response in onFailure, if there is a total failure or node, we do get this call in listener's onFailure. Based on this call we reset the nodeEntry present in the cache. So, new calls can go through now for this node.
In upcoming change : As we'll have shardEntry in the cache (mostly concurrent), if a request is timed out or we received a full failure from node, here on leader side, we'll reset the shardEntry so it can be re-fetched by next set of calls (until then new calls won't be made for those shards, only after we update this cache{source of truth for us}).

But the response for first request r1 arrived later after r2

Not possible, as discussed above. If the shardId is same, we'll send the new request, only after marking previous one as failed or updating the common cache object as restartFetching.

In this case, old fetch round responses should be discarded.

They will be discarded, current logic does it for nodeEntries, we'll do same thing for shardEntries. Only change is - today in per shard cache, we used to mark one entry of HM as fetching=false, now we'll need to do it for a batch of entries of HM for each shard in our newly created per node cache.

But now consider the case from Point 2 where 2 fetch requests were sent with different shardId list to same node and both responses should be processed. How are both the cases differentiated and handled?

As the cache contain different entries for every shard, two different request for different set of shards can easily update their respective set of entries in the map and we'll update the response only after all the shards are fetched from that particular node.

As you are making calls per node and response structure is already changed, then do you still need explicit changes for DiscoveryNode, that should be addressed automatically?

Yes, partially count of DiscoveryNode overall in memory will be automatically reduced as number of responses are reduced. Basically, if we're returning the responses from 1 node only, we don't need to keep the BaseNodeResponse class as parent class of response object. Because we know it's the same DiscoveryNode object for all shards (which are being sent in 1 single request).
As I mentioned previously https://github.com/opensearch-project/OpenSearch/issues/5098#issuecomment-1596722825

As first solution itself is giving some relief in number of DiscoveryNode objects, we can debate more on if this is truly required based on gains diff of part 1 and part 2.

Phase 2 will only be applicable in case we set the batchSize as 100 and there are 1000 shards on the node. Now we may end up calling new transport 10 times and get 10 DiscoveryNode objects for the same node. So, to optimize even that we could see if we can just keep it 1 DiscoveryNode object per node and not store every response's DiscoveryNode object in memory if ephemeralId or uniqueId for the node is same(then it can be referred from or updated in some cache maintained on cluster manager side).

Be careful when changing ExistingShardsAllocator interface to ensure it is not a breaking change as it is exposed via ClusterPlugin

For now I only see one implementation and this method is not overridden there. I think if a new ExistingShardsAllocator implementation is provided by new plugins, that need to add extra methods which we're planning to add now (allocation of shards in a batch per node).

What will be the behavior during mixed mode upgrades would it send batch request to new nodes and per shard to old nodes?

Handling of this will be difficult and make the code complicated (imagine all book keeping done for both type of requests). So it'll be easier to enable batched request only when all nodes have the handler for batch request. Else we'll need to write more code just to handle the different type of responses and merge their responses to get a final view :)

will it fetch only for failed shards or all the shards in that batch? What is the current retry mechanism?

Current retry mechanism marks which node returned failure, based on that in next round only those nodes are picked up. For failures, we'll mark only specific shards as failed in cache if only some of those are failed while fetching from a node. We won't fail the whole batch. Whole batch will be failed only in case node itself return failure or gets disconnected.

Today, the cache can get per shard view easily, Now it has to iterate over all entries in the top level map with NodeId as the key to check the shard status. Right?

Cache is only being used for book keeping purpose today to know which nodes are fetching or fetching is done/failed etc. We also store the result in cache that is a map where key is nodeId and value is NodeEntry so for each node where we're sending the request we keep the result in cache. Then overall decision is made base on this map.
So, yes it's true that when we batch the request for a single node, our response won't have all the necessary data for making an allocation decision. For that we'll need to read the full data : responses of every "per node batched request" to actually get a view of an unassigned shard and all the nodes it got in the different responses.
For example :
response-from-n1 = {S1: s1-res-n1, S2 : s2-res-n1}
response-from-n2 = {S1: s1-res-n2, S2 : s2-res-n2}
Now for making an actual allocation decision, we'll need a view of
S1 -> [s1-res-n1, s2-res-n2]
So, we do need to traverse outer node level map and build internal shard level view to make final decision (as it's done today in PSA).
Ref :
https://github.com/opensearch-project/OpenSearch/blob/87a833fbedfbcf9029d5e14e0597f8c5e8968086/server/src/main/java/org/opensearch/gateway/PrimaryShardAllocator.java#L132-L182

Thanks @amkhar for collating the knowledge and attaching my Draft PR for it. So here are few points that I will like to clarify:

We'll keep getting new set of shards which are unassigned if more nodes go down. For that we need to maintain an atomic set where shards are maintained for which fetch is already happening and async success call will clear it out from the set. This way we always make sure that we can cleanly send a new request for different set of shards if new nodes fail. Before sending a call, we must check and get the diff using that atomic/synchronized set.

I think you mean to say the diff should be between the atomic set you maintain where fetch is in being progress and the Unassigned Shards list maintained by RoutingNodes here. So once fetch gets completed for current batch, we clear it from our atomic set. And if the Allocators(PSA/RSA) are unable to make decision with current available data, shards will still be in unassigned state. And the next round of batch(reroute call) will pick it as a diff.

In case result was failed for a particular shard (as request is being executed for single node it's hard to see partial failures, but still..), we'll either restart fetching based on retryable exceptions or permanently fail the fetch as it is done today.

In current scenario:
There are two categories of failures
1- physical failures(FailedNodeException), 2- failures while fetching the metadata from shards.
1- First type of failures happen, when during the the transport any catastrophic event happens. These failures are restarted again by doing another round of reroute
https://github.com/opensearch-project/OpenSearch/blob/17ee1ce11d7b4382418454b9319679b9a5e4cbe9/server/src/main/java/org/opensearch/gateway/AsyncShardFetch.java#L184-L189
2- For second failures, The respone NodeGatewayStartedShard itself store the exception
https://github.com/opensearch-project/OpenSearch/blob/17ee1ce11d7b4382418454b9319679b9a5e4cbe9/server/src/main/java/org/opensearch/gateway/TransportNodesListGatewayStartedShards.java#L349-L353
These exception are handled by PSA/RSA accordingly.
https://github.com/opensearch-project/OpenSearch/blob/17ee1ce11d7b4382418454b9319679b9a5e4cbe9/server/src/main/java/org/opensearch/gateway/PrimaryShardAllocator.java#L344-L361
We should not change this existing logic, since even if we have batch, in case of catestrophic physical Failures( category-1) we need to restart entire batch, because that node will come under FailedNodeException hierarchy.
Category 2 will be handled by PSA/RSA as it is done now.

@shwetathareja @amkhar I still think that our main culprit here is GatewayAllocator fetching part which eats memory and has overheard of more number of transport calls. Current @amkhar solution is trying to revamp the Assignment part which is not our culprit.
We could make separation of concerns here by separating fetching part and Abstracting it out in different new interface.
We build the view of fetching (with the current proposed way of @amkhar that handles failures and have retries, i.e better version of my draft PR) and send the data to PSA/RSA per shard as it is done today.
If you look high level, the data collection part is still abstracted from PSA/RSA. By my idea are changing the abstraction layer of fetching from single shard to batch of shards.
This way will save from making any change in current interfaces and abstract classes and avoid any breaking changes.

For now I only see one implementation and this method is not overridden there. I think if a new ExistingShardsAllocator implementation is provided by new plugins, that need to add extra methods which we're planning to add now (allocation of shards in a batch per node).

@amkhar There could be custom plugin implementation that customers might be using in their private repos. And, you can't introduce a breaking change in 2.x where they need to implement a new method in the interface.

A for loop is required to run it for multiple shards, I don't think running this in parallel new threads is a good idea.

Today, it runs in parallel for all unassigned shard by the virtue of per shard calls. What is the latency overhead for fetch this detail on data node per shard?

For now I only see one implementation and this method is not overridden there. I think if a new ExistingShardsAllocator implementation is provided by new plugins, that need to add extra methods which we're planning to add now (allocation of shards in a batch per node).

@amkhar There could be custom plugin implementation that customers might be using in their private repos. And, you can't introduce a breaking change in 2.x where they need to implement a new method in the interface.

One suggestion you can run a loop on existing allocateUnassigned method from current interface in loop as the default implementation for the new batch method in ExistingShardsAllocator. That ways you can move to batch method without breaking any clients.

Benchmarking result of POC code[#7269 ]
Version: Opensearch 2.3
Number of data nodes: 50
Number of master nodes: 3
Heap for node(commited): 5GB
Java version : Java 11

We are reducing the number of Response Objects of the Transport Calls since we are now making less number of transport calls altogether

Overall Heap decreased for DisoveryNodeObject objects reduction due to batching to number of data nodes
Older code

Poc Code:

Number of cache objects reduced to number of data nodes.
Older Code:

POC code:

@shwetathareja

@amkhar There could be custom plugin implementation that customers might be using in their private repos. And, you can't introduce a breaking change in 2.x where they need to implement a new method in the interface.

True, will provide a default implementation. For example if new method if allocateAllUnassignedShards, it'll internally just loop for each shard and hit the existing allocateUnassigned method. That will be the same flow which is there today.

Few more updates:

AllocationService will just pass RoutingAllocation object via ExistingShardsAllocator interface's new method.
Batching will be done in GatewayAllocator. GA will iterate over RoutingAllocation object and create batches of configured size. It'll also maintain maps to find out the correct batch for a particular shard.
So every time a jumbo list of shards is given to GA, it'll traverse and find existing batches or create new batches out of those shards. And trigger fetchBatchData method of all those batch level fetcher.
Batch level fetcher will be responsible for maintaining the cache for a particular set of shard and that will be fixed.
Request structure will just start sending a list of Shards and response will give us the data for all those shards. Same List response will be stored as Generic in cache object (as it is stored today for just a single shard)
So we'll be sending the batch of shards to all data nodes.
While returning the data to PSA/RSA by AsyncShardFetchBatch (batch level fetcher), we'll adapt the data. So, rest of the code of PSA/RSA working on per shard level data can work as it is.
As soon as all shards in a batch are started, GA will free up that batch level fetcher from its map that will ultimately free up the cache object that fetcher class is holding.
Node level failures will still be handled similarly as they are done today.
Shard level failures also will be handled similarly.

Note : Introducing this BATCH_SIZE configuration to reduce blast radius, and start assignment as soon as we get the data for a small batch of shards instead of all the shards (as done in POC code).
Tradeoff: with multiple batches in flight, there will be total (Batch Count x Node Count) DiscoveryNode objects in memory in comparison to Node Count DiscoveryNode objects if a single batch is used.
Downside with single batch : Single batch code changes are more complex as we need to manage the cache and keep appending the newly unassigned shard in same object. Also clearing up the cache is hard and not efficient as we can't simply remove the Single batch level fetcher object until all shards are STARTED.

@shwetathareja
We need to decide about how to merge the PRs of this bigger project. Order of PRs in my understanding is going to be like this : https://github.com/opensearch-project/OpenSearch/issues/5098#issuecomment-1651229185

This is the order of PRs which I think we'll follow for merging in main.

What is the backport plan? Will each of these PRs be safe to backport to 2.x immediately upon merging to main?

@amkhar What is the backport plan? Will each of these PRs be safe to backport to 2.x immediately upon merging to main?

@andrross
Yes, as most of the PRs are new classes, it should be safe to merge them to 2.x. Even the last PR which will contain the triggering code, should be safe to backport.
If I realize any hiccups, will bring that to attention.

Also, can you tag these issues/PRs with the intended version (like v2.10.0), or let me know if you don't have permissions to add labels.

I've added all subtasks of this bigger project here :

#8098

I don't have permissions to add labels, yes intended version is 2.10.0. Please attach that label or add permissions for me to attach labels.

@vikasvb90 @dhwanilpatel  - requesting for feedback on the PRs.


next-intl beta for Next 13 not redirect to base URL when default locale match

Description
I have config defaultLocale in middleware.js but when a use Link component to change to default language always use language preffix instead base url
Reproduction URL (CodeSandbox)
https://codesandbox.io/p/sandbox/modest-jang-1yhdgq?file=%2Fapp%2F[locales]%2Fpage.js&selection=[{"endColumn"%3A24%2C"endLineNumber"%3A8%2C"startColumn"%3A24%2C"startLineNumber"%3A8}]
Reproduction description

Open preview sandbox in new browser tab.
Click link to change language to english
Click link to change language to spanish

Expected behaviour
When to change to english (is not default locale) the url is https://xxxx:3000/en, but when you change again to spanish (default language) the url si http://xxxx:3000/es, when the correct behaviour could must be http://xxxx:3000

Thanks for the report! I made a note of this in https://github.com/amannn/next-intl/pull/149 and will look into it!

This is fixed in 2.12.0-beta.3!

This is fixed in 2.12.0-beta.3!

@amannn I'm using<EMAIL_ADDRESS>and this issue is still occuring
default locale: 'fr'
From page: /blog
<Link href="/blog" locale="fr"/>
href is '/blog' ( Valid )
From page: /en/blog
<Link href="/blog" locale="fr"/>
href is '/fr/blog' ( Invalid )


WIREFRAME:  Advanced Filtering

As a developer, I'd like to be able to filter the Work Item list using only text syntax to make advanced filters.
Systems that do this:

Github
Jira

Ideas from the team:
Max A: i would suggest treat the field in the combo to be an "advanced query" string and parse it to make it do the RightThing(tm)
Michael K: I would think that when I click on a icon/button on the filter bar, the "labels" are collapsing to an expression that can be edited. Like in the file managers that use breadcrumps. Hitting CTRL-L collapses the breadcrumbs to an expression/path.

Samir has been working on design improvements fro advanced filtering.  Please add links to your work as you progress.

First draft created is available here https://redhat.invisionapp.com/share/QABI3D5MX#/screens
Got initial feedback from Catherine. Have started working on creating examples based on a simplified version shortly. Meanwhile please share your feedback on the current draft version.
This initial draft version focuses on a few things to do the heavy lifting like --

Filtering based on nested parameters
Advanced  filtering options like AND / OR relationships among parameters
Optionally providing an approach to customise most used/popular filters up front etc.

Basically there 2 types of information we might deal with across the application  -- the data itself (the backlog items, files, staus, authors, etc. ) and the second is the set of meta data or the secondary layer of info. like the tags . The tags can have multi levels of relationships.  The proposed interaction was trying to do the heavy lifting across multi-levels of information hierarchy among the metadata and can be customised to fit in the simplest use case also where you just need one linear hierarchy of information and just one level of meta-data.
The "preferred filters" are placed with a goal to be accessed upfront. There can tweaks like use of just icons, that are commonly understood... or replacing radio-boxes with toggle states. Keeping "preferred filters" as separate block will help in well position them in responsive layout in smaller resolutions. They can appear as next to the text field however it has to be upfront and should not be in one level down for access.

@catrobson : I am adding your feedback for reference with my comments inline [] :
Thanks for putting together this first draft!  I took a look through it.  At first pass, it feels complex to understand for the end user with the different components.
[SAMIR: agree that too many components can be simplified. will look into that aspect. may be we can remove the customisation section out ]
One thing that might help make it more consumable to me, and possibly others, is if you add a real example of how the Planner team might use this multi-faceted filtering capability to create a real filter they need for their backlog, and show a wireframe with realistic data based on that.  Its hard to tell whether it would pull together and make more sense once there was real data in it.
[SAMIR: will be providing some real life example in next version ... lets say using WI or build status context]
Another observation I had was that It felt like having two "areas" to the filter - the box w/ has advanced features, and the right hand preferred filters as another interaction separate from the filter dropdown was a bit confusing.  What do you think about having everything fairly co-located?
[SAMIR: yeah..fundamentally they are two different components, visually we can put them together with closer proximity ... let's explore]
Also consider that I don't think we would use this type of filtering component on something like codebases where we have a pretty simple list.  I expect we could stay with a "simple filter" from PatternFly for this, whereas the Planner list view is much more complex and needs a much richer interaction for filtering.  With the planner view, you could say that the left hand panel (Iterations, Backlog) is a "filter" and that we could build out that to include more rich filters.  I could see us having "All" (all work items), "Backlog" (currently statically defined filter, but we could let people configure what backlog means to them), "My Named Filter 1", "My Named filter 2",etc. before the iterations listing - or pulling the iterations into a "view" of that left hand panel that is slightly different from the work item filtered list views that are system-created or self-created.
One feature that I know Todd wants us to have that I did not see in this is a way for me to "Save" my filter as a "quick filter" for future use.  JIRA and gmail have this type of a feature if you want to see how others do it.
[SAMIR: yes..this is an user need i missed out ..will add]

Attached the draft 0.2 to invision for the advance filtering.
Please review and share your thoughts. https://redhat.invisionapp.com/share/GUBPEPBCW
(Also you can get at google slide https://docs.google.com/a/redhat.com/presentation/d/1fZf3VZbvujz3tcGweuIz1phlp0MvVLFKUlwWoZDx6ZI/edit?usp=sharing)
@catrobson @muruGanesan, @Essjaysee , @mindreeper2420 , @bdellasc, @joshuawilson , @aslakknutsen

There is a call with Matt Carrano tonight, who is working on a PF solution on advance filter for BPM. Will try to get his views /thoughts on this approch and if there is any other clues I get from any other members who attend the call on the same to ensure, we cover the all critical user-needs covered.

Had a discussion with last evening on PF advance filter gaps and the typical needs/use-cases with Matt Carrano, Liz Clayton, Ming Zhong,  Matthew Stevens. Some thoughts from the call:

The proposed solution does fill the gaps pointed out.
In some usecases the users may need some advanced features more than that is offered in quick filter, but they may not need complex query building. - though this is an assumption and further usecases are being looked into.
In such scenario, the proposed solution approach can be used till where it opens the popup to modify the filters with relationships..
The query builder can be plugged into wherever required and can be exlored with another story.

Looking at these here are my takeaways:

The proposed solution can be split into 2 stories - 1 . for a filter that can handle both simple and medium complexities of queries (i.e. the prosed solution excluding the query builder)  2. the later will be another story that can specifically define the query builder part  and both of them can be 2 different modules.

Basically the current proposed solution combines both the needs.
The first part will be sufficient to close the current story  and the later will help creating a new PF widget for query building.
Share your thoughts/views.

I have updated the wireframe and have uploaded to the new version 0.3 to Invision.
Please review and share your thoughts.
https://redhat.invisionapp.com/d/main#/projects/prototypes/11063451
Source at Google drive: https://docs.google.com/a/redhat.com/presentation/d/1R5DUlOgILODh4NUsrZzO3fhREhHliHOWyFx1QDim82U/edit?usp=sharing

Sprint 131c:  Focus on smaller screen sizes and how all the elements will fit into the filter header bar with changes that Brian is introducing with #401.

Updated to show how it will behave in lower resolutions
Also how it will fit in to the new layout designed by Brian #401.
Check the updated version 0.4 at
https://redhat.invisionapp.com/share/X6BYQRBVN
Primarily review these slides #42, #43, #44
@catrobson @muruGanesan, @Essjaysee , @mindreeper2420 , @bdellasc, @joshuawilson , @aslakknutsen


Update http4s-blaze-client, ... to 0.23.15

About this PR
📦 Updates

org.http4s:http4s-blaze-client
org.http4s:http4s-blaze-server

from 0.21.34 to 0.23.15
📜 GitHub Release Notes
Usage
✅ Please merge!
I'll automatically update this PR to resolve conflicts as long as you don't change it yourself.
If you'd like to skip this version, you can just close this PR. If you have any feedback, just mention me in the comments below.
Configure Scala Steward for your repository with a .scala-steward.conf file.
Have a fantastic day writing Scala!

💡 Applied Scalafix Migrations

org.http4s:http4s-.*:0.22.0 (created no change)

github:http4s/http4s/v0_22?sha=series/0.22

⚙ Adjust future updates
Add this to your .scala-steward.conf file to ignore future updates of this dependency:
updates.ignore = [ { groupId = "org.http4s" } ]

Or, add this to slow down future updates of this dependency:
dependencyOverrides = [{
  pullRequests = { frequency = "30 days" },
  dependency = { groupId = "org.http4s" }
}]

labels: library-update, early-semver-major, semver-spec-minor, version-scheme:early-semver, scalafix-migrations, commit-count:1

Superseded by #1607.