L1nq0 commented on PR #9075: URL: https://github.com/apache/storm/pull/9075#issuecomment-5555570287
No problem. However, could you please take another look at #9076? I've already completed the second push. The previous maintainer would prefer to merge #9076 first before starting work on #9075. reiabreu ***@***.***> 于 2026年9月6日周日 上午12:53写道： > *reiabreu* left a comment (apache/storm#9075) > <https://github.com/apache/storm/pull/9075#issuecomment-5553324579> > > hey! > @rzo1 <https://github.com/rzo1> already covered all important points and > it's a +1 on his review. > > I did a sweep with the help of an LLM and this is what is outstanding: > > 1. Add maxarray (and maxdepth/maxrefs) to the sample pattern, not just > maxbytes. maxbytes only counts bytes already read, so one big array still > gets allocated before the limit kicks in. The class deny-list does nothing > for this. > 2. Say that this filter replaces a JVM-wide -Djdk.serialFilter, it > does not add to it. The default factory just returns the new filter, so a > filter set in worker.childopts is quietly dropped on this path. Either > merge with the existing one via ObjectInputFilter.merge(...), or document > it. > 3. Factory scope (rzo1 #3 <https://github.com/apache/storm/pull/3>): > fix it by documenting, not by moving the install into > SerializationFactory.getKryo(). The filter can only go where the > ObjectInputStream is opened, which is inside SerializableSerializer. That > works for the default factory because it's our code and it builds that > serializer. A custom topology.kryo.factory is a black box: > SerializationFactory only gets back a finished Kryo, with no way to reach > its fallback path. So a generic install can't cover custom factories. Best > to just state in the Config javadoc and both docs that the filter covers > the default factory's fallback bridge, and a custom factory is on its own. > 4. Minor: maxbytes is per object, not per tuple (a new > ObjectInputStream is opened per value). Worth saying so in the docs. > > — > Reply to this email directly, view it on GitHub > <https://github.com/apache/storm/pull/9075?email_source=notifications&email_token=BWIRIJGVOBY3H77SYLVPCN35NRAHDA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNJVGMZTENBVG442M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-5553324579>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/BWIRIJEZ4W7WL5763WHM4ET5NRAHDAVCNFSNUABEKJSXA33TNF2G64TZHMYTIMJTGU2DOMB3JFZXG5LFHM2TGNBUGQ2DMNBWG6QXMAQ> > . > You are receiving this because you authored the thread.Message ID: > ***@***.***> > -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
