-
Notifications
You must be signed in to change notification settings - Fork 20.4k
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
swarm sync fixes #3780
swarm sync fixes #3780
Conversation
@zelig, thanks for your PR! By analyzing the history of the files in this pull request, we identified @karalabe, @mchusovlianov and @fjl to be potential reviewers. |
Thank you for your contribution! Your commits seem to not adhere to the repository coding standards
Please check the contribution guidelines for more details. This message was auto-generated by https://gitcop.com |
1 similar comment
Thank you for your contribution! Your commits seem to not adhere to the repository coding standards
Please check the contribution guidelines for more details. This message was auto-generated by https://gitcop.com |
utils.Fatalf("--datadir must be specificed") | ||
} | ||
|
||
sourcedatadirfull = fmt.Sprintf("%s/swarm/bzz-%s", path.Clean(importdatadir), importbzzaccount) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this mean that <source-bzzaccount>
is a required command line argument? Otherwise the source directory will be <importdir>/swarm/bzz-
which won't exist?
syncRequestMsg // 0x04 | ||
deliveryRequestMsg // 0x05 | ||
unsyncedKeysMsg // 0x06 | ||
paymentMsg // 0x07 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this necessary? Will it not cause confusion between old and new nodes misinterpreting the meaning of the codes?
swarm/network/syncer.go
Outdated
@@ -32,6 +33,7 @@ const ( | |||
requestDbBatchSize = 512 // size of batch before written to request db | |||
keyBufferSize = 1024 // size of buffer for unsynced keys | |||
syncBatchSize = 128 // maximum batchsize for outgoing requests | |||
historyBufferSize = 128 // maximum batchsize for outgoing requests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment needs updating (it is copied from the line above)
I don't think this is done yet. There is still a matter of the last batch of keys not being sent by syncer. You'll be putting it here I presume, @zelig ? |
Thank you for your contribution! Your commits seem to not adhere to the repository coding standards
Please check the contribution guidelines for more details. This message was auto-generated by https://gitcop.com |
f935bd1
to
0ee8038
Compare
Thank you for your contribution! Your commits seem to not adhere to the repository coding standards
Please check the contribution guidelines for more details. This message was auto-generated by https://gitcop.com |
0ee8038
to
5564bb0
Compare
Thank you for your contribution! Your commits seem to not adhere to the repository coding standards
Please check the contribution guidelines for more details. This message was auto-generated by https://gitcop.com |
5564bb0
to
442bf23
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 0-returning distance function in line 33 of cleandb.go is definitely dangerous and probably wrong. Pease do not approve before this is properly handled.
Thank you for your contribution! Your commits seem to not adhere to the repository coding standards
Please check the contribution guidelines for more details. This message was auto-generated by https://gitcop.com |
Thank you for your contribution! Your commits seem to not adhere to the repository coding standards
Please check the contribution guidelines for more details. This message was auto-generated by https://gitcop.com |
3 similar comments
Thank you for your contribution! Your commits seem to not adhere to the repository coding standards
Please check the contribution guidelines for more details. This message was auto-generated by https://gitcop.com |
Thank you for your contribution! Your commits seem to not adhere to the repository coding standards
Please check the contribution guidelines for more details. This message was auto-generated by https://gitcop.com |
Thank you for your contribution! Your commits seem to not adhere to the repository coding standards
Please check the contribution guidelines for more details. This message was auto-generated by https://gitcop.com |
Postponed to 1.6.1 as it wasn't ready in time for the release. |
The Using the scripts from the swarm-dev branch I span up a 9 node cluster, uploaded some data and restarted the cluster:
I then observed the error in the logs for
Looking at the invalid chunk on
|
28c6a55
to
44bcce0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you provide a concise explanation of the problem that this PR fixes?
swarm/storage/types_test.go
Outdated
// You should have received a copy of the GNU Lesser General Public License | ||
// along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>. | ||
|
||
package storage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is empty.
swarm/storage/database.go
Outdated
@@ -29,6 +29,7 @@ import ( | |||
) | |||
|
|||
const openFileLimit = 128 | |||
//const openFileLimit = -1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this one.
swarm/storage/chunker_test.go
Outdated
// func BenchmarkSplitPyramid_5(t *testing.B) { benchmarkSplitPyramid(100000, t) } | ||
// func BenchmarkSplitPyramid_6(t *testing.B) { benchmarkSplitPyramid(1000000, t) } | ||
// func BenchmarkSplitPyramid_7(t *testing.B) { benchmarkSplitPyramid(10000000, t) } | ||
// func BenchmarkSplitPyramid_8(t *testing.B) { benchmarkSplitPyramid(100000000, t) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove or uncomment.
thanks @fjl pushing the changes |
186e42e
to
54a7faf
Compare
54a7faf
to
b7a08cb
Compare
* new index and structure in db_store * syncronisation: iterator more efficient * syncer history sync is simplified * swarm/api: fix tests NOTE: this represents an interim fix until network layer rewrite is complete Update cleandb.go Just a comment, not to forget fixing this. Update dbstore.go Added a TODO comment to refactor the database constructor. swarm/storage: Adding bucketed chunk counters to chunk database cmd/swarm: cleandb now takes proper PO function based on bzzaccount swarm/storage: fix bucketcount stats in dbstore swarm/storage, cmd/swarm: add db dump as command
* increment dataindex read from db; fixes invalid hash error * increment last index; fixes repeated keys in history sync * only use MaxPO (=8) proxbins in dbstore; simplifies iterations * reduce logging output and make it useful * fix tests for fuse and http API * simplify chunkstore tests in common_test, streamline tests for dbstore, memstore and add benchmarks
b7a08cb
to
73cc315
Compare
Thank you for your contribution! Your commits seem to not adhere to the repository coding standards
Please check the contribution guidelines for more details. This message was auto-generated by https://gitcop.com |
This PR provdes fixes to various incorrect behaviours and inefficiencies in the syncer and
it fixes the mysterious hash integrity bug in the dbstore
6|<proximity_order_as_byte>|<dataIdx>
so can iterate by prox bin for syncing