Closed bhalevy closed 1 year ago
thank you for the fix Benny! i am very sorry for the regression. will review this fix early tomorrow.
also, shall we decrease the default memory reserved for UDF if we are not testing UDF in the tests? or at least make it a run-time configurable setting -- we could configured it using environmental variable at least.
also, shall we decrease the default memory reserved for UDF if we are not testing UDF in the tests? or at least make it a run-time configurable setting -- we could configured it using environmental variable at least.
It seems like we can safely keep the default when guaranteeing (512 - 50)MB per, so no need to ATM
thank you for the fix Benny! i am very sorry for the regression. will review this fix early tomorrow.
It's something I wanted to get done for a long time and needed strong enough motivation, so thanks for providing it! :)
@fruch why?
I was about to merge, and Github UI kindly suggested.
but not for any specific reason
i guess that Benny's branch was out of sync with the target branch (next
), so github kindly showed the merge button so maintainer / contributor is allowed to bring the fork updated by merging the upstream's HEAD back to the fork. and the button was just tempting :-)
In v2 (b234b36):
process_opts
now returns the result as OrderedDict
rather than mutating an external variableIn 29c35dd:
--scylla-manager
rather than --scylla-manager=
as the processing of the option removes that =
char. plus we want to ignore the whole class of options starting with --scylla-manager, if more to come.@fruch what version of python do we use for running the CI tests? It complains about:
TypeError: unsupported operand type(s) for |: 'collections.OrderedDict' and 'collections.OrderedDict'
but it works well for me using Python 3.11.2
.
I can change the code to use update()
but I like the |
operator better
@fruch what version of python do we use for running the CI tests? It complains about:
TypeError: unsupported operand type(s) for |: 'collections.OrderedDict' and 'collections.OrderedDict'
but it works well for me using
Python 3.11.2
. I can change the code to useupdate()
but I like the|
operator better
the CI is running with:
platform linux -- Python 3.8.16, pytest-7.2.2, pluggy-1.0.0
dtest has move to python 3.11 just recently, but ccm is being used also in multiple drivers CI, I don't want to also break those. not without verifying they are all ready for it.
i think we are using python 3.8 in scylla-ccm's CI, see https://github.com/scylladb/scylla-ccm/blob/9a353befee7e589846503568a0e6720fa56a27fe/.github/workflows/integration-tests.yml#L22. but since we are using python3.11 on fedora 37 for CI testing in scylladb/scylladb, see https://jenkins.scylladb.com/job/scylla-master/job/scylla-ci/88/console:
03:14:33 ============================= test session starts ==============================
03:14:33 platform linux -- Python 3.11.1, pytest-6.2.5, py-1.11.0, pluggy-1.0.0 -- /usr/bin/python3
03:14:33 cachedir: .pytest_cache
so probably we should test scylla-ccm using python3.11 to be consistent with where it is mostly used? i can help with this if this is right direction.
EDIT, just saw the reply.
In ddad8a9:
|
operator to unify options from SCYLLA_EXT_ENV and jmv_argsIn c4a5215:
_mem_set_during_test
renamed to _mem_mb_set_during_test
to better reflect what it means
We'd like to use the values provided in the SCYLLA_EXT_OPTS environment variable as defaults, but also derive self._mem_mb_per_cpu from them.
Then, if a test passes --smp, without --memory in jvm_args we should calculate --memory from self._mem_mb_per_cpu * _smp.
Otherwise, the default --memory parameter given in SCYLLA_EXT_OPTS could be too small if the test uses more shards than the default.
See for example https://jenkins.scylladb.com/view/master/job/scylla-master/job/dtest-daily-release/216/artifact/logs-full.release.018/dtest-gw2.log that times out when bootstrapping new nodes with smp=8 and memory=1024M takes increasingly longer due to the immense memory pressure.
https://jenkins.scylladb.com/view/master/job/scylla-master/job/dtest-daily-release/216/artifact/logs-full.release.018/1678691413194_lwt_schema_modification_test.py%3A%3ATestLWTSchemaModification%3A%3Atest_lwt_load/node1.log There are lots and lots of memory pressure indications, in the form of
This apparently started happening after scylladb/scylladb@020483aa594c0978bd3696c0d2b316fb77db5b2e That changed:
Increasing the overall memory reservation for wasm and bringing scylla to its knees with 1024M total for 8 shards.