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.

Sign up for GitHub

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

Jump to bottom

Storing ledger metadata in binary format #723

Closed
sijie opened this issue Nov 14, 2017 · 1 comment
Closed

Storing ledger metadata in binary format #723

sijie opened this issue Nov 14, 2017 · 1 comment
Assignees
@ivankelly
Labels
area/metadata area/protocol type/feature
Milestone
4.9.0

Comments

@sijie
Copy link
Member

sijie commented Nov 14, 2017

FEATURE REQUEST

  1. Please describe the feature you are requesting.

Currently we stores ledger metadata in TextFormat protobuf not in binary format. Text format doesn't have a good backwards and forwards compatibility story as binary format. We should switch from text format to binary format.

In order to keep backward compatibility and maintain good debuggability as text format, we can consider writing both text format and binary format.

  1. Indicate the importance of this issue to you (blocker, must-have, should-have, nice-to-have). Are you currently using any workarounds to address this issue?

must-have in 4.7

  1. Provide any additional detail on your proposed use case for this feature.
@sijie sijie added this to the 4.7.0 milestone Nov 14, 2017
@sijie sijie modified the milestones: 4.7.0, 4.8.0 Mar 12, 2018
@eolivelli eolivelli modified the milestones: 4.8.0, 4.9.0 Sep 6, 2018
@ivankelly ivankelly self-assigned this Nov 29, 2018
ivankelly added a commit to ivankelly/bookkeeper that referenced this issue Nov 29, 2018
@ivankelly
Move serialization code out of LedgerMetadata
b5ffc0c 
This PR moves the serialization code out of LedgerMetadata so that it
can later be adapted to run different serialization code depending on
the environment.

Notable non-refactor changes:
- LedgerMetadata#toString no longer uses #serialize because it's no
  longer available. Instead it uses the ToString helper from guava.
  byte[] fields are now base64 encoded.
- There's a new state enum and getter in api.LedgerMetadata. This is
  so that LedgerMetadataFormat can be removed from
  client.LedgerMetadata.

Master issue: apache#723
@ivankelly ivankelly mentioned this issue Nov 29, 2018
Merged
ivankelly added a commit that referenced this issue Nov 30, 2018
@ivankelly
Move serialization code out of LedgerMetadata
d6007ed 
This PR moves the serialization code out of LedgerMetadata so that it
can later be adapted to run different serialization code depending on
the environment.

Notable non-refactor changes:
- LedgerMetadata#toString no longer uses #serialize because it's no
  longer available. Instead it uses the ToString helper from guava.
  byte[] fields are now base64 encoded.
- There's a new state enum and getter in api.LedgerMetadata. This is
  so that LedgerMetadataFormat can be removed from
  client.LedgerMetadata.

Master issue: #723


Reviewers: Sijie Guo <sijie@apache.org>

This closes #1848 from ivankelly/refactor-md-serde
ivankelly added a commit to ivankelly/bookkeeper that referenced this issue Nov 30, 2018
@ivankelly
Users of LedgerMetadata should use the api interface
2efe5f4 
All users to LedgerMetadata should use api.LedgerMetadata rather than
client.LedgerMetadata.

Some methods have been promoted to the interface to allow this.

Other methods have been moved out to a utility class that acts purely
on api.LedgerMetadata.

client.LedgerMetadata has been renamed to LedgerMetadataImpl.

Master issue: apache#723
@ivankelly ivankelly mentioned this issue Nov 30, 2018
Merged
ivankelly added a commit that referenced this issue Dec 3, 2018
@ivankelly
Users of LedgerMetadata should use the api interface
8e0e9fb 
All users to LedgerMetadata should use api.LedgerMetadata rather than
client.LedgerMetadata.

Some methods have been promoted to the interface to allow this.

Other methods have been moved out to a utility class that acts purely
on api.LedgerMetadata.

client.LedgerMetadata has been renamed to LedgerMetadataImpl.

Master issue: #723


Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Sijie Guo <sijie@apache.org>

This closes #1852 from ivankelly/kill-store-ctime
ivankelly added a commit to ivankelly/bookkeeper that referenced this issue Dec 5, 2018
@ivankelly
Ledger manager factories initialized with max metadata version
e0cd41d 
In dce4fd4 the max metadata format version was put in the
configuration which was then used to create ledger managers. This is
not safe though, as it there are multiple paths which through which
ledger manager factories are created and some may not see this
configuration modification. For example, on a new cluster where there
is no preexisting layout, the layout isn't written until after the
ledger manager factory has been created.

This patch changes LedgerManagerFactory#initialize to explicitly
require that the max ledger metadata format is specified in the call.

Master issue: apache#723
ivankelly added a commit to ivankelly/bookkeeper that referenced this issue Dec 5, 2018
@ivankelly
HTTP GetMetaService should use #toString rather than #serialize
4b0e194 
Once the binary ledger metadata serializing is available, the output
of GetMetaService would no longer be understandable. In preparation
for this, make GetMetaService use #toString, rather than calling

Master issue: apache#723
ivankelly added a commit to ivankelly/bookkeeper that referenced this issue Dec 5, 2018
@ivankelly
Ledger manager factories initialized with max metadata version
54603ba 
In dce4fd4 the max metadata format version was put in the
configuration which was then used to create ledger managers. This is
not safe though, as it there are multiple paths which through which
ledger manager factories are created and some may not see this
configuration modification. For example, on a new cluster where there
is no preexisting layout, the layout isn't written until after the
ledger manager factory has been created.

This patch changes LedgerManagerFactory#initialize to explicitly
require that the max ledger metadata format is specified in the call.

Master issue: apache#723
This was referenced Dec 5, 2018
Merged
Merged
ivankelly added a commit that referenced this issue Dec 6, 2018
@ivankelly
Ledger manager factories initialized with max metadata version
dd684b3 
In dce4fd4 the max metadata format version was put in the
configuration which was then used to create ledger managers. This is
not safe though, as it there are multiple paths which through which
ledger manager factories are created and some may not see this
configuration modification. For example, on a new cluster where there
is no preexisting layout, the layout isn't written until after the
ledger manager factory has been created.

This patch changes LedgerManagerFactory#initialize to explicitly
require that the max ledger metadata format is specified in the call.

Master issue: #723

Reviewers: Enrico Olivelli <eolivelli@gmail.com>

This closes #1865 from ivankelly/explicit-max
ivankelly added a commit to ivankelly/bookkeeper that referenced this issue Dec 6, 2018
@ivankelly
Binary metadata format
781533c 
This patch adds a binary metadata format and bumps the metadata format
version to 3. The contents of the binary metadata is the same as the
contents of the text format for now. The difference is that the binary
is more compact, and the fields can be added to the metadata when
using the binary format, which isn't possible with the text
format. With the text format, parsing with a client that didn't
recognise the new field would fail.

For now, the text format (version 2) is still used by default. We will
provide a tool to allow administrators to bump to version 3.

Some tests have been modified to provide digest and password to the
builder. All protobuf metadata in released versions has had digest and
password (first protobuf metadata was in release-4.2.0). So if new
metadata is created or read with version 2, it will have this two
fields set.

Master issue: apache#723
@ivankelly ivankelly mentioned this issue Dec 6, 2018
Merged
ivankelly added a commit to ivankelly/bookkeeper that referenced this issue Dec 19, 2018
@ivankelly
Revert maxLedgerMetadataFormatVersion changes in layout
d396cb3 
There is ongoing discussions about how to do this, so I'm reverting this
change for now to allow 4.9 release to proceed. The change modify the
contents of the layout znode, so once they're in an official release they
cannot be removed without breaking BC.

The reverted changes are:

dd684b Ledger manager factories initialized with max metadata version
dce4fd Add max ledger metadata format version to layout

Master issue: apache#723
@ivankelly ivankelly mentioned this issue Dec 19, 2018
Merged
sijie pushed a commit that referenced this issue Dec 21, 2018
@ivankelly @sijie
Revert maxLedgerMetadataFormatVersion changes in layout
e921193 
There is ongoing discussions about how to do this, so I'm reverting this
change for now to allow 4.9 release to proceed. The change modifies the
contents of the layout znode, so once they're in an official release they
cannot be removed without breaking BC.

The reverted changes are:

dd684b Ledger manager factories initialized with max metadata version
dce4fd Add max ledger metadata format version to layout

Master issue: #723


Reviewers: Sijie Guo <sijie@apache.org>, Enrico Olivelli <eolivelli@gmail.com>

This closes #1890 from ivankelly/revert-max-format
ivankelly added a commit to ivankelly/bookkeeper that referenced this issue Jan 8, 2019
@ivankelly
HTTP GetMetaService returns JSON rather than calling #serialize
8052c8e 
Once the binary ledger metadata serializing is available, the output
of GetMetaService would no longer be understandable. In preparation
for this, make GetMetaService use output JSON, rather than directly
calling #serialize.

Master issue: apache#723
ivankelly added a commit to ivankelly/bookkeeper that referenced this issue Jan 8, 2019
@ivankelly
HTTP GetMetaService returns JSON rather than calling #serialize
be9a976 
Once the binary ledger metadata serializing is available, the output
of GetMetaService would no longer be understandable. In preparation
for this, make GetMetaService use output JSON, rather than directly
calling #serialize.

Master issue: apache#723
sijie pushed a commit that referenced this issue Jan 17, 2019
@ivankelly @sijie
Binary metadata format
5343665 
This patch adds a binary metadata format and bumps the metadata format
version to 3. The contents of the binary metadata is the same as the
contents of the text format for now. The difference is that the binary
is more compact, and the fields can be added to the metadata when
using the binary format, which isn't possible with the text
format. With the text format, parsing with a client that didn't
recognise the new field would fail.

For now, the text format (version 2) is still used by default. We will
provide a tool to allow administrators to bump to version 3.

Some tests have been modified to provide digest and password to the
builder. All protobuf metadata in released versions has had digest and
password (first protobuf metadata was in release-4.2.0). So if new
metadata is created or read with version 2, it will have this two
fields set.

Master issue: #723


Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Matteo Merli <mmerli@apache.org>, Sijie Guo <sijie@apache.org>

This closes #1866 from ivankelly/binary-metadata
sijie pushed a commit that referenced this issue Jan 21, 2019
@ivankelly @sijie
HTTP GetMetaService returns JSON rather than calling #serialize
19a598e 
Once the binary ledger metadata serializing is available, the output
of GetMetaService would no longer be understandable. In preparation
for this, make GetMetaService use output JSON, rather than directly
calling #serialize.

Master issue: #723


Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Sijie Guo <sijie@apache.org>

This closes #1864 from ivankelly/get-to-string
@sijie
Copy link
Member Author

sijie commented Jan 21, 2019

All the necessary changes are in master and will be shipped as part of 4.9.0.

@sijie sijie closed this as completed Jan 21, 2019
athanatos pushed a commit to athanatos/bookkeeper that referenced this issue Jan 25, 2019
@anmolnar
ZOOKEEPER-3202: Add timing margin to improve reliability of testClien…
14eefcb 
…tServerSSL()

Allowing just 5 seconds for 3 quorum peers to start and elect a leader is a bit tight, at least when running 4 test processes in parallel inside a (Linux) Docker container on a (non-Linux) laptop.  Add up to 10 seconds of extra margin.

Author: Michael Edwards <Michael Edwards>

Reviewers: andor@apache.org

Closes apache#723 from mkedwards/ZOOKEEPER-3202
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ivankelly ivankelly

Labels
area/metadata area/protocol type/feature
Projects
None yet
Milestone
4.9.0
Development

No branches or pull requests
3 participants
@ivankelly @sijie @eolivelli