Skip to content

Conversation

@dkropachev
Copy link
Collaborator

@dkropachev dkropachev commented Nov 18, 2025

There are certain rules you need to following in the cases when dicts are modified and read in parallel.
Otherwise you end up with RuntimeError: dictionary changed size during iteration. Rules are the following:

  1. Any iteration over items or keys, needs to be done over a snapshot, i.e. list() or set()
  2. Avoid unnecessary iterations like len(d.keys()), you can replace them with len(d)

This commit fixes code to match these rules in the pool.py and metrics.py

Fixes: #594

Pre-review checklist

  • I have split my patch into logically separate commits.
  • All commit messages clearly explain what they change and why.
  • I added relevant tests for new features and bug fixes.
  • All commits compile, pass static checks and pass test.
  • PR description sums up the changes and reasons why they should be introduced.
  • I have provided docstrings for the public items that I want to introduce.
  • I have adjusted the documentation in ./docs/source/.
  • I added appropriate Fixes: annotations to PR description.

@dkropachev dkropachev requested review from Lorak-mmk, fruch and sylwiaszunejko and removed request for sylwiaszunejko November 18, 2025 11:33
@dkropachev dkropachev self-assigned this Nov 18, 2025
@mykaul
Copy link

mykaul commented Nov 18, 2025

Typo in commit title, otherwise looks OK.

There are certain rules you need to following in the cases when dicts are modified and read in parallel.
Otherwise you end up with `RuntimeError: dictionary changed size during iteration`.
Rules are the following:
1. Any iteration over items or keys, needs to be done over a snapshot,
   i.e. `list()` or `set()`
2. Avoid unnecessary iterations like `len(d.keys())`, you can replace
   them with `len(d)`

This commit fixes code to match these rules in the `pool.py` and
`metrics.py`
@dkropachev dkropachev force-pushed the dk/fix-dicts-handling branch from e90948b to 51fbb77 Compare November 18, 2025 12:44
@dkropachev
Copy link
Collaborator Author

Typo in commit title, otherwise looks OK.

Fixed, pipeline is fialing because of the weird network issue:

 Trying to resolve the latest version from remote
  (node:2094) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
  (Use `node --trace-deprecation ...` to show where the warning was created)
  Error: Java setup failed due to network issue or timeout: error code: 500
  Error: error code: 500

No changes to the code done since last cicd run.

@dkropachev dkropachev merged commit 2509b1c into scylladb:master Nov 18, 2025
9 of 23 checks passed
@fruch
Copy link

fruch commented Nov 18, 2025

I would guess cloudflare issues, half of the internet was broken today cause of those

Typo in commit title, otherwise looks OK.

Fixed, pipeline is fialing because of the weird network issue:

 Trying to resolve the latest version from remote
  (node:2094) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
  (Use `node --trace-deprecation ...` to show where the warning was created)
  Error: Java setup failed due to network issue or timeout: error code: 500
  Error: error code: 500

No changes to the code done since last cicd run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TestShardAware.test_advanced_shard_aware_port may fail

5 participants