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

ZBUG-4409: Missing Blobs on a restored account from backup with S3 storage #1698

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions store/src/java/com/zimbra/cs/store/StoreManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.zimbra.cs.store.file.FileBlobStore;
import com.zimbra.cs.store.helper.ClassHelper;
import com.zimbra.cs.util.Zimbra;
import com.zimbra.cs.volume.Volume;
import com.zimbra.cs.volume.VolumeManager;

import java.io.IOException;
Expand Down Expand Up @@ -292,6 +293,20 @@ public abstract Blob storeIncoming(InputStream data, boolean storeAsIs)
public abstract StagedBlob stage(InputStream data, long actualSize, Mailbox mbox)
throws IOException, ServiceException;

/**
* Stage an incoming <code>InputStream</code> to an
* appropriate place for subsequent storage in a <code>Mailbox</code> via
* {@link #link(StagedBlob, Mailbox, int, int)} or {@link #renameTo}.
*
* @param data the data stream
* @param actualSize the content size, or {@code -1} if the content size is not available
* @param callback callback, or {@code null}
* @param mbox the mailbox
* @param volume the volume
*/
public abstract StagedBlob stage(InputStream data, long actualSize, Mailbox mbox, Volume volume)
throws IOException, ServiceException;

/**
* Stage an incoming <code>InputStream</code> to an
* appropriate place for subsequent storage in a <code>Mailbox</code> via
Expand All @@ -306,6 +321,21 @@ public StagedBlob stage(InputStream data, Mailbox mbox)
return stage(data, -1, mbox);
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of hard code value can we declare variable for -1 , because its confusing

}

/**
* Stage an incoming <code>InputStream</code> to an
* appropriate place for subsequent storage in a <code>Mailbox</code> via
* {@link #link(StagedBlob, Mailbox, int, int)} or {@link #renameTo}.
*
* @param data the data stream
* @param callback callback, or {@code null}
* @param mbox the mailbox
* @param volume the volume
*/
public StagedBlob stage(InputStream data, Mailbox mbox, Volume volume)
throws IOException, ServiceException {
return stage(data, -1, mbox, volume);
}

/**
* Stage an incoming <code>Blob</code> (see {@link #storeIncoming}) to an
* appropriate place for subsequent storage in a <code>Mailbox</code> via
Expand Down
15 changes: 15 additions & 0 deletions store/src/java/com/zimbra/cs/store/external/ExternalBlobIO.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,21 @@ public interface ExternalBlobIO {
*/
String writeStreamToStore(InputStream in, long actualSize, Mailbox mbox) throws IOException, ServiceException;

/**
* Write data to blob store
*
* @param in: InputStream containing data to be written
* @param actualSize: size of data in stream, or -1 if size is unknown. To be used by implementor for optimization where possible
* @param mbox: Mailbox which contains the blob. Can optionally be used by store for partitioning
* @param destVolId: Destination Volume id on which we need to store the blob
* @return locator string for the stored blob, unique identifier created by storage protocol
* @throws IOException
* @throws ServiceException
*/
default String writeStreamToStore(InputStream in, long actualSize, Mailbox mbox, short destVolId) throws IOException, ServiceException {
return null;
}
Gopal-Moolchandani marked this conversation as resolved.
Show resolved Hide resolved

/**
* Create an input stream for reading data from blob store
* @param locator: identifier string for the blob as returned from write operation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import com.zimbra.cs.store.StagedBlob;
import com.zimbra.cs.store.StoreManager;
import com.zimbra.cs.store.file.VolumeMailboxBlob;
import com.zimbra.cs.volume.Volume;

/**
* Abstract base class for external store integration.
Expand Down Expand Up @@ -272,6 +273,31 @@ public StagedBlob stage(Blob blob, Mailbox mbox) throws IOException, ServiceExce
}
}

public StagedBlob stage(Blob blob, Mailbox mbox, Volume volume) throws IOException, ServiceException {
if (supports(StoreFeature.RESUMABLE_UPLOAD) && blob instanceof ExternalUploadedBlob) {
ZimbraLog.store.debug("blob already uploaded, just need to commit");
String locator = ((ExternalResumableUpload) this).finishUpload((ExternalUploadedBlob) blob);
if (locator != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (locator != null) {
if (null != locator) {

ZimbraLog.store.debug("wrote to locator %s",locator);
preshitaagrawal marked this conversation as resolved.
Show resolved Hide resolved
localCache.put(locator, getContent(blob));
} else {
ZimbraLog.store.warn("blob staging returned null locator");
}
return new ExternalStagedBlob(mbox, blob.getDigest(), blob.getRawSize(), locator);
} else {
InputStream is = getContent(blob);
try {
StagedBlob staged = stage(is, blob.getRawSize(), mbox, volume);
if (staged != null && staged.getLocator() != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (staged != null && staged.getLocator() != null) {
if (null != staged && null != staged.getLocator()) {

localCache.put(staged.getLocator(), getContent(blob));
}
return staged;
} finally {
ByteUtil.closeStream(is);
}
}
}

@Override
public StagedBlob stage(InputStream in, long actualSize, Mailbox mbox) throws ServiceException, IOException {
if (actualSize < 0) {
Expand Down Expand Up @@ -303,7 +329,35 @@ public StagedBlob stage(InputStream in, long actualSize, Mailbox mbox) throws Se
}
}

public StagedBlob stage(InputStream in, long actualSize, Mailbox mbox, Volume volume) throws ServiceException, IOException {
if (actualSize < 0) {
Blob blob = storeIncoming(in);
try {
return stage(blob, mbox, volume);
} finally {
quietDelete(blob);
}
}
MessageDigest digest;
try {
digest = MessageDigest.getInstance("SHA-256");
} catch (NoSuchAlgorithmException e) {
throw ServiceException.FAILURE("SHA-256 digest not found", e);
}
ByteUtil.PositionInputStream pin = new ByteUtil.PositionInputStream(new DigestInputStream(in, digest));

try {
String locator = writeStreamToStore(pin, actualSize, mbox, volume.getId());
if (locator != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (locator != null) {
if (null != locator) {

ZimbraLog.store.debug("wrote to locator %s",locator);
Gopal-Moolchandani marked this conversation as resolved.
Show resolved Hide resolved
} else {
ZimbraLog.store.warn("blob staging returned null locator");
}
return new ExternalStagedBlob(mbox, ByteUtil.encodeFSSafeBase64(digest.digest()), pin.getPosition(), locator);
} catch (IOException e) {
throw ServiceException.FAILURE("unable to stage blob", e);
}
}
@Override
public Blob storeIncoming(InputStream data, boolean storeAsIs) throws IOException,
ServiceException {
Expand Down
6 changes: 6 additions & 0 deletions store/src/java/com/zimbra/cs/store/file/FileBlobStore.java
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,12 @@ public VolumeStagedBlob stage(InputStream in, long actualSize, Mailbox mbox)
return new VolumeStagedBlob(mbox, (VolumeBlob) blob).markStagedDirectly();
}

@Override
public StagedBlob stage(InputStream data, long actualSize, Mailbox mbox, Volume volume) throws IOException, ServiceException {
// mailbox store is on the same volume as incoming directory, so no need to stage the blob
throw ServiceException.FAILURE("Operation can not be completed because ExternalStoreManager is not available", null);
Copy link
Contributor

Choose a reason for hiding this comment

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

It is not necessary this method will be always executed for external storage, this message should be generic

}

@Override
public VolumeStagedBlob stage(Blob blob, Mailbox mbox) throws IOException {
// mailbox store is on the same volume as incoming directory, so no need to stage the blob
Expand Down