Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implement block iterator #56

Merged
merged 7 commits into from
Nov 28, 2023
Merged

Conversation

ronoaldo
Copy link
Contributor

@ronoaldo ronoaldo commented Feb 20, 2023

WIP.

Todo:

  • Return a Closer to allow early stop from the iterator caller
  • Rename to AsBlockPos
  • Fix infinite loop on error
  • Improve assertions for testing to avoid false positives

database/Sql.Rows.Close() should be called as it is
a safe to call and idempotent; not calling it will keep the
underlying connection open.

Added an integration test to reproduce the issue
and a fix to it.

Fixes minetest-go#53
The iterator returns a go channel to allow for
concurrent access over all the data in the map,
skipping any non-existing blocks.

Fix minetest-go#55
@coveralls
Copy link

Pull Request Test Coverage Report for Build 4237033961

  • 61 of 73 (83.56%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.04%) to 86.638%

Changes Missing Coverage Covered Lines Changed/Added Lines %
block/block_postgres.go 27 33 81.82%
block/block_sqlite.go 31 37 83.78%
Totals Coverage Status
Change from base Build 4231376287: -0.04%
Covered Lines: 603
Relevant Lines: 696

💛 - Coveralls

Copy link
Contributor Author

@ronoaldo ronoaldo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost good! Just a few important QOL changes, as well as better testing.

block/block.go Outdated Show resolved Hide resolved
block/block.go Outdated Show resolved Hide resolved
block/block_postgres.go Outdated Show resolved Hide resolved
block/block_test.go Show resolved Hide resolved
Fixed an infinite loop/invalid results returned by
the Iterator if database/data is corrupted.
Implemented the Closer type and methods to allow
for callers to early stop the iteretor when they're done.
@ronoaldo
Copy link
Contributor Author

Ready for review/comments! I am planning to use this to perhaps allow parallel execution of the Export and Clean implementations of https://github.com/minetest-go/mapcleaner

@ronoaldo ronoaldo marked this pull request as ready for review November 28, 2023 00:58
@BuckarooBanzay
Copy link
Member

nice, lgtm 👍

@BuckarooBanzay BuckarooBanzay merged commit 990b478 into minetest-go:master Nov 28, 2023
1 check passed
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.

3 participants