Skip to content

nsqd: refactor storage engine#1170

Open
andyxning wants to merge 1 commit into
nsqio:masterfrom
andyxning:refactor_nsqd_storage_engine
Open

nsqd: refactor storage engine#1170
andyxning wants to merge 1 commit into
nsqio:masterfrom
andyxning:refactor_nsqd_storage_engine

Conversation

@andyxning

@andyxning andyxning commented Jul 26, 2019

Copy link
Copy Markdown
Member

Fix #1139
Fix #1137

@andyxning andyxning force-pushed the refactor_nsqd_storage_engine branch 8 times, most recently from 7fbab14 to a593551 Compare July 28, 2019 08:37
@andyxning

Copy link
Copy Markdown
Member Author

/cc @mreiferson @ploxiln This PR is ready for review. PTAL.

@andyxning andyxning changed the title WIP: refactor nsqd storage engine refactor nsqd storage engine Jul 28, 2019
@andyxning andyxning force-pushed the refactor_nsqd_storage_engine branch from a593551 to 29bd197 Compare July 28, 2019 09:16
@ploxiln

ploxiln commented Jul 28, 2019

Copy link
Copy Markdown
Member

what's not great about github.com/vmihailenco/msgpack is that it has a bunch of dependencies, including google.golang.org/appengine, and that pulls in github.com/golang/protobuf ...

@andyxning

andyxning commented Jul 29, 2019

Copy link
Copy Markdown
Member Author

what's not great about github.com/vmihailenco/msgpack is that it has a bunch of dependencies, including google.golang.org/appengine, and that pulls in github.com/golang/protobuf ...

@ploxiln I think it deserves adding the these dependencies since msgpack will make it faster and denser than json or plain bytes. This will store more messages on the same disk and write to disk more messages.

Actually, when the functionality grows, the project will get more and more complex such as logic and dependency. Kubernetes is an example.

@andyxning

Copy link
Copy Markdown
Member Author

/cc @ploxiln @mreiferson

2 similar comments
@andyxning

Copy link
Copy Markdown
Member Author

/cc @ploxiln @mreiferson

@andyxning

Copy link
Copy Markdown
Member Author

/cc @ploxiln @mreiferson

@andyxning

Copy link
Copy Markdown
Member Author

/cc @jehiah

@andyxning

Copy link
Copy Markdown
Member Author

/cc @jehiah @mreiferson @ploxiln

1 similar comment
@andyxning

Copy link
Copy Markdown
Member Author

/cc @jehiah @mreiferson @ploxiln

@andyxning

Copy link
Copy Markdown
Member Author

/cc @jehiah @mreiferson @ploxiln Anyone can help to take a look at this? It has waited for more than 24 days. :(

@ploxiln

ploxiln commented Aug 23, 2019

Copy link
Copy Markdown
Member

There are msgpack libraries which do not have dependencies.

While msgpack is more efficient than json, especially for numbers and byte buffers, this is still using strings for keys, instead of a single byte. So, I'm still not a fan of this approach.

@andyxning

Copy link
Copy Markdown
Member Author

While msgpack is more efficient than json, especially for numbers and byte buffers, this is still using strings for keys, instead of a single byte. So, I'm still not a fan of this approach.

Using single byte for keys will force us to manage parsing keys manually and it is obviously error-prone. This is even make the parsing logic more complicated when a new field added. By using json or msgpack we offload this parsing logic to json or msgpack and they have done quite well in marshal and unmarshal in normal cases and abnormal cases such as some field loss or newly added. We need a tradeoff between maintainability and storage efficiency. In this case, i prefer maintainability.

Btw, @ploxiln thanks for reviewing this. Can you reach @mreiferson @jehiah personally for informing them help to review this PR. This PR has been made for more than 30d. :(

@ploxiln

ploxiln commented Aug 26, 2019

Copy link
Copy Markdown
Member

@mreiferson recently started at a new job/employer, and is waiting to get official authorization to contribute to NSQ

@andyxning

Copy link
Copy Markdown
Member Author

OK, let's be more patient. :)

@andyxning

Copy link
Copy Markdown
Member Author

@mreiferson Ping.

@andyxning

Copy link
Copy Markdown
Member Author

@ploxiln @mreiferson We should work on this PR for now. :)

@andyxning

andyxning commented Sep 18, 2019

Copy link
Copy Markdown
Member Author

We need to push this PR forward. @ploxiln Do you know who is also actively working on nsq. We can discuss this on slack or mailing list. Anyway, we need to support this ASAP. This has been delayed by more than one month. :(

@andyxning

Copy link
Copy Markdown
Member Author

@ploxiln @jehiah @mreiferson Can you please take a look at this.

Internally, i have upgraded a cluster and everything works fine.

@ploxiln

ploxiln commented Nov 8, 2019

Copy link
Copy Markdown
Member

But you're running a fork of nsq 0.3.8 right? So it doesn't really benefit your organization much if this is merged?

(I'm curious why 0.3.8 - which of the interfaces removed in 1.0 are you dependent on?)

@andyxning

Copy link
Copy Markdown
Member Author

which of the interfaces removed in 1.0 are you dependent on?

Nothing. We just run 0.3.8 well and nothing new feature in versions after 0.3.8 we need.

@andyxning

andyxning commented Nov 11, 2019

Copy link
Copy Markdown
Member Author

But you're running a fork of nsq 0.3.8 right? So it doesn't really benefit your organization much if this is merged?

Yes. But i think with this PR nsqd can support more complicated features easily, such as the compression feature.

@mreiferson mreiferson changed the title refactor nsqd storage engine nsqd: refactor storage engine Jun 14, 2020
@yusank

yusank commented Jan 20, 2021

Copy link
Copy Markdown

Hi, is there any plan to release the new version of nsq with this new storage engine? kind need this feature.

@EvansJahja

Copy link
Copy Markdown

Hi @ploxiln @jehiah @mreiferson
We would benefit from this feature as well and it seems this has been dragging for over 2 years now. Are there any plans on continuing on this?

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

nsqd: refactor NSQD storage backend format

5 participants