Include size of snapshot in snapshot metadata #29602

vladimirdolzhenko · 2018-04-19T06:17:10Z

Adds difference of number of files (and file sizes) between prev and current snapshot. Total number/size reflects total number/size of files in snapshot.

There are inconsistent properties - "number_of_files" but "total_size" and in fact they reflect incremental change. Align all stats property names and add effective total files properties.

To summarize the change:

stats (for op like GET /_snapshot/backup1/snapshot_2/_status?human) looks

before:

"stats": {
"number_of_files": 8,
"processed_files": 8,
"total_size": "4.6kb",
"total_size_in_bytes": 4797,
"processed_size": "4.6kb",
"processed_size_in_bytes": 4797,
"start_time_in_millis": 1523882347327,
"time": "225ms",
"time_in_millis": 225
}

after:

{
    "stats": {
        "incremental": {
        	"file_count": 8,
        	"size": 4797,
        	"size_in_bytes": "4.6kb"
        },
       "processed": {
        	"file_count": 7,
        	"size": 4000,
        	"size_in_bytes": "4.0kb"
       },
        "total": {
        	"file_count": 11,
        	"size": "8kb",
        	"size_in_bytes": 4797
        },
        "start_time_in_millis": 1523882347327,
        "time": "225ms",
        "time_in_millis": 225,
        <!-- bwc section -->
       "number_of_files": 8,
       "processed_files": 8,
       "total_size": "4.6kb",
       "total_size_in_bytes": 4797,
       "processed_size": "4.6kb",
       "processed_size_in_bytes": 4797
    }
}

Note: processed is visible while snapshot is processing - it goes away when snapshot is done.

elasticmachine · 2018-04-19T06:46:56Z

Pinging @elastic/es-distributed

Adds difference of number of files (and file sizes) between prev and current snapshot. Total number/size reflects total number/size of files in snapshot. Closes elastic#18543

ywelsch

Thanks @vladimirdolzhenko. While looking at the details of this PR, I've noticed that we don't need to serialize the total size / total number of files to disk, the information is already readily available (this will also simplify BWC). We can compute it in the constructor of BlobStoreIndexShardSnapshot based on the indexFiles that are provided as parameter to it.

For BWC purposes, we will then only have to worry about SnapshotStats. Here we have IndexingIt.testUpdateSnapshotStatus that runs a snapshot in a mixed-version cluster, so ensures that nothing is horribly wrong with the BWC of the writeTo/readFrom logic.

In terms of naming, I think we should do the following changes:

call the new field incremental_size instead of difference_of_size. We already use the "incremental" terminology in the S/R docs.
call the new field incremental_file_count instead of difference_of_number_of_files.

I would be even inclined to rename number_of_files/total_number_of_files to total_file_count

Finally, once we agree on the naming, we will also have to document this in the "breaking changes" documentation, and also add a note to the new change log.

ywelsch · 2018-04-19T07:44:00Z

...r/src/main/java/org/elasticsearch/index/snapshots/blobstore/BlobStoreIndexShardSnapshot.java

@@ -549,7 +587,14 @@ public static BlobStoreIndexShardSnapshot fromXContent(XContentParser parser) th
                }
            }
        }
+        // for the sake of backward compatibility
+        if (totalNumberOfFiles < 0){


spacing between ) and { here and a few lines below

ywelsch · 2018-04-19T07:54:48Z

server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/status/SnapshotStats.java

    }

    static final class Fields {
        static final String STATS = "stats";
-        static final String NUMBER_OF_FILES = "number_of_files";
+        static final String DIFFERENCE_OF_NUMBER_OF_FILES = "difference_of_number_of_files";
+        static final String TOTAL_NUMBER_OF_FILES = "total_number_of_files";


just renaming this field should break serialization as fromXContent and toXContent now use different field names.

ywelsch · 2018-04-19T09:20:49Z

server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/SnapshotBlocksIT.java

@@ -84,6 +100,90 @@ protected void setUpRepository() throws Exception {
        ensureSearchable();
    }

+    public void testSnapshotTotalAndDifferenceSizes() throws IOException {


this test does not really fit into this class (it's about blocks). Can you move it to DedicatedClusterSnapshotRestoreIT?

ywelsch

I've added more comments.

The CI failure on the PR is also real, see the output:

[2018-04-20T01:06:08,507][INFO ][o.e.b.MixedClusterClientYamlTestSuiteIT] [test {p0=snapshot.status/10_basic/Get snapshot status}]: after test
15:08:00 FAILURE 0.19s | MixedClusterClientYamlTestSuiteIT.test {p0=snapshot.status/10_basic/Get snapshot status} <<< FAILURES!
15:08:00    > Throwable #1: java.lang.AssertionError: Failure at [snapshot.status/10_basic:29]: expected [2xx] status code but api [snapshot.status] returned [400 Bad Request] [{"error":{"root_cause":[{"type":"parse_exception","reason":"unknown parameter [incremental_file_count]","stack_trace":"ElasticsearchParseException[unknown parameter [incremental_file_count]]\n\tat org.elasticsearch.index.snapshots.blobstore.BlobStoreIndexShardSnapshot.fromXContent(BlobStoreIndexShardSnapshot.java:534)\n\tat org.elasticsearch.repositories.blobstore.BlobStoreFormat.read(BlobStoreFormat.java:115)\n\tat org.elasticsearch.repositories.blobstore.ChecksumBlobStoreFormat.readBlob(ChecksumBlobStoreFormat.java:114)\n\tat org.elasticsearch.repositories.blobstore.BlobStoreFormat.read(BlobStoreFormat.java:90)\n\tat org.elasticsearch.repositories.blobstore.BlobStoreRepository$Context.loadSnapshot(BlobStoreRepository.java:943)\n\tat org.elasticsearch.repositories.blobstore.BlobStoreRepository.getShardSnapshotStatus(BlobStoreRepository.java:836)\n\tat org.elasticsearch.snapshots.SnapshotsService.snapshotShards(SnapshotsService.java:622)\n\tat org.elasticsearch.action.admin.cluster.snapshots.status.TransportSnapshotsStatusAction.buildResponse(TransportSnapshotsStatusAction.java:234)\n\tat org.elasticsearch.action.admin.cluster.snapshots.status.TransportSnapshotsStatusAction.masterOperation(TransportSnapshotsStatusAction.java:99)\n\tat org.elasticsearch.action.admin.cluster.snapshots.status.TransportSnapshotsStatusAction.masterOperation(TransportSnapshotsStatusAction.java:61)\n\tat org.elasticsearch.action.support.master.TransportMasterNodeAction.masterOperation(TransportMasterNodeAction.java:88)\n\tat org.elasticsearch.action.support.master.TransportMasterNodeAction$AsyncSingleAction$2.doRun(TransportMasterNodeAction.java:174)\n\tat org.elasticsearch.common.util.concurrent.ThreadContext$ContextPreservingAbstractRunnable.doRun(ThreadContext.java:724)\n\tat org.elasticsearch.common.util.concurrent.AbstractRunnable.run(AbstractRunnable.java:37)\n\tat java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)\n\tat java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)\n\tat java.lang.Thread.run(Thread.java:748)\n"}],"type":"parse_exception","reason":"unknown parameter [incremental_file_count]","stack_trace":"ElasticsearchParseException[unknown parameter [incremental_file_count]]\n\tat org.elasticsearch.index.snapshots.blobstore.BlobStoreIndexShardSnapshot.fromXContent(BlobStoreIndexShardSnapshot.java:534)\n\tat org.elasticsearch.repositories.blobstore.BlobStoreFormat.read(BlobStoreFormat.java:115)\n\tat org.elasticsearch.repositories.blobstore.ChecksumBlobStoreFormat.readBlob(ChecksumBlobStoreFormat.java:114)\n\tat org.elasticsearch.repositories.blobstore.BlobStoreFormat.read(BlobStoreFormat.java:90)\n\tat org.elasticsearch.repositories.blobstore.BlobStoreRepository$Context.loadSnapshot(BlobStoreRepository.java:943)\n\tat org.elasticsearch.repositories.blobstore.BlobStoreRepository.getShardSnapshotStatus(BlobStoreRepository.java:836)\n\tat org.elasticsearch.snapshots.SnapshotsService.snapshotShards(SnapshotsService.java:622)\n\tat org.elasticsearch.action.admin.cluster.snapshots.status.TransportSnapshotsStatusAction.buildResponse(TransportSnapshotsStatusAction.java:234)\n\tat org.elasticsearch.action.admin.cluster.snapshots.status.TransportSnapshotsStatusAction.masterOperation(TransportSnapshotsStatusAction.java:99)\n\tat org.elasticsearch.action.admin.cluster.snapshots.status.TransportSnapshotsStatusAction.masterOperation(TransportSnapshotsStatusAction.java:61)\n\tat org.elasticsearch.action.support.master.TransportMasterNodeAction.masterOperation(TransportMasterNodeAction.java:88)\n\tat org.elasticsearch.action.support.master.TransportMasterNodeAction$AsyncSingleAction$2.doRun(TransportMasterNodeAction.java:174)\n\tat org.elasticsearch.common.util.concurrent.ThreadContext$ContextPreservingAbstractRunnable.doRun(ThreadContext.java:724)\n\tat org.elasticsearch.common.util.concurrent.AbstractRunnable.run(AbstractRunnable.java:37)\n\tat java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)\n\tat java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)\n\tat java.lang.Thread.run(Thread.java:748)\n"},"status":400}]
15:08:00    > 	at __randomizedtesting.SeedInfo.seed([44DD9D016CD36F30:CC89A2DBC22F02C8]:0)
15:08:00    > 	at org.elasticsearch.test.rest.yaml.ESClientYamlSuiteTestCase.executeSection(ESClientYamlSuiteTestCase.java:343)
15:08:00    > 	at org.elasticsearch.test.rest.yaml.ESClientYamlSuiteTestCase.test(ESClientYamlSuiteTestCase.java:320)
15:08:00    > 	at java.lang.Thread.run(Thread.java:748)
15:08:00    > Caused by: java.lang.AssertionError: expected [2xx] status code but api [snapshot.status] returned [400 Bad Request] [{"error":{"root_cause":[{"type":"parse_exception","reason":"unknown parameter [incremental_file_count]","stack_trace":"ElasticsearchParseException[unknown parameter [incremental_file_count]]\n\tat org.elasticsearch.index.snapshots.blobstore.BlobStoreIndexShardSnapshot.fromXContent(BlobStoreIndexShardSnapshot.java:534)\n\tat org.elasticsearch.repositories.blobstore.BlobStoreFormat.read(BlobStoreFormat.java:115)\n\tat org.elasticsearch.repositories.blobstore.ChecksumBlobStoreFormat.readBlob(ChecksumBlobStoreFormat.java:114)\n\tat org.elasticsearch.repositories.blobstore.BlobStoreFormat.read(BlobStoreFormat.java:90)\n\tat org.elasticsearch.repositories.blobstore.BlobStoreRepository$Context.loadSnapshot(BlobStoreRepository.java:943)\n\tat org.elasticsearch.repositories.blobstore.BlobStoreRepository.getShardSnapshotStatus(BlobStoreRepository.java:836)\n\tat org.elasticsearch.snapshots.SnapshotsService.snapshotShards(SnapshotsService.java:622)\n\tat org.elasticsearch.action.admin.cluster.snapshots.status.TransportSnapshotsStatusAction.buildResponse(TransportSnapshotsStatusAction.java:234)\n\tat org.elasticsearch.action.admin.cluster.snapshots.status.TransportSnapshotsStatusAction.masterOperation(TransportSnapshotsStatusAction.java:99)\n\tat org.elasticsearch.action.admin.cluster.snapshots.status.TransportSnapshotsStatusAction.masterOperation(TransportSnapshotsStatusAction.java:61)\n\tat org.elasticsearch.action.support.master.TransportMasterNodeAction.masterOperation(TransportMasterNodeAction.java:88)\n\tat org.elasticsearch.action.support.master.TransportMasterNodeAction$AsyncSingleAction$2.doRun(TransportMasterNodeAction.java:174)\n\tat org.elasticsearch.common.util.concurrent.ThreadContext$ContextPreservingAbstractRunnable.doRun(ThreadContext.java:724)\n\tat org.elasticsearch.common.util.concurrent.AbstractRunnable.run(AbstractRunnable.java:37)\n\tat java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)\n\tat java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)\n\tat java.lang.Thread.run(Thread.java:748)\n"}],"type":"parse_exception","reason":"unknown parameter [incremental_file_count]","stack_trace":"ElasticsearchParseException[unknown parameter [incremental_file_count]]\n\tat org.elasticsearch.index.snapshots.blobstore.BlobStoreIndexShardSnapshot.fromXContent(BlobStoreIndexShardSnapshot.java:534)\n\tat org.elasticsearch.repositories.blobstore.BlobStoreFormat.read(BlobStoreFormat.java:115)\n\tat org.elasticsearch.repositories.blobstore.ChecksumBlobStoreFormat.readBlob(ChecksumBlobStoreFormat.java:114)\n\tat org.elasticsearch.repositories.blobstore.BlobStoreFormat.read(BlobStoreFormat.java:90)\n\tat org.elasticsearch.repositories.blobstore.BlobStoreRepository$Context.loadSnapshot(BlobStoreRepository.java:943)\n\tat org.elasticsearch.repositories.blobstore.BlobStoreRepository.getShardSnapshotStatus(BlobStoreRepository.java:836)\n\tat org.elasticsearch.snapshots.SnapshotsService.snapshotShards(SnapshotsService.java:622)\n\tat org.elasticsearch.action.admin.cluster.snapshots.status.TransportSnapshotsStatusAction.buildResponse(TransportSnapshotsStatusAction.java:234)\n\tat org.elasticsearch.action.admin.cluster.snapshots.status.TransportSnapshotsStatusAction.masterOperation(TransportSnapshotsStatusAction.java:99)\n\tat org.elasticsearch.action.admin.cluster.snapshots.status.TransportSnapshotsStatusAction.masterOperation(TransportSnapshotsStatusAction.java:61)\n\tat org.elasticsearch.action.support.master.TransportMasterNodeAction.masterOperation(TransportMasterNodeAction.java:88)\n\tat org.elasticsearch.action.support.master.TransportMasterNodeAction$AsyncSingleAction$2.doRun(TransportMasterNodeAction.java:174)\n\tat org.elasticsearch.common.util.concurrent.ThreadContext$ContextPreservingAbstractRunnable.doRun(ThreadContext.java:724)\n\tat org.elasticsearch.common.util.concurrent.AbstractRunnable.run(AbstractRunnable.java:37)\n\tat java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)\n\tat java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)\n\tat java.lang.Thread.run(Thread.java:748)\n"},"status":400}]
15:08:00    > 	at org.elasticsearch.test.rest.yaml.section.DoSection.execute(DoSection.java:240)
15:08:00    > 	at org.elasticsearch.test.rest.yaml.ESClientYamlSuiteTestCase.executeSection(ESClientYamlSuiteTestCase.java:336)
15:08:00    > 	... 37 more

You might be able to reproduce this test failure by running ./gradlew :qa:mixed-cluster:v6.3.0-SNAPSHOT#mixedClusterTestRunner

It means that once a snapshot with version 7.0.0 has been written, version 6.3 will not be able to load information about that snapshot anymore (due to the BlobStoreIndexShardSnapshot parse fields being renamed). I wonder if we should leave the internal serialization field names be, as it's not strictly needed to break this.

ywelsch · 2018-04-20T06:55:04Z

server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/status/SnapshotStats.java

@@ -105,29 +126,45 @@ public void writeTo(StreamOutput out) throws IOException {
        out.writeVLong(startTime);
        out.writeVLong(time);

-        out.writeVInt(numberOfFiles);
-        out.writeVInt(processedFiles);
+        out.writeVInt(totalFileCount);


shouldn't this be the incrementalFileCount?

ywelsch · 2018-04-20T06:55:26Z

server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/status/SnapshotStats.java

-        out.writeVInt(numberOfFiles);
-        out.writeVInt(processedFiles);
+        out.writeVInt(totalFileCount);
+        out.writeVInt(processedFileCount);

        out.writeVLong(totalSize);


Shouldn't this be incrementalSize now?

ywelsch · 2018-04-20T06:56:01Z

server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/status/SnapshotStats.java

+
+        if (out.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) {
+            out.writeVInt(incrementalFileCount);
+            out.writeVLong(incrementalSize);


Shouldn't this be totalFileCount and totalSize here? (That's the new stats that we've added)

ywelsch · 2018-04-20T06:56:29Z

server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/status/SnapshotStats.java

+            incrementalSize = in.readVLong();
+        } else {
+            incrementalFileCount = totalFileCount;
+            incrementalSize = totalSize;


same comments here as above.

ywelsch · 2018-04-20T07:05:39Z

...r/src/main/java/org/elasticsearch/index/snapshots/blobstore/BlobStoreIndexShardSnapshot.java

+        int totalFileCount = 0;
+        long totalSize = 0;
+        for (FileInfo indexFile : indexFiles) {
+            totalFileCount ++;


just this.totalFileCount = indexFiles.size() ? Do we even need to store this in an extra field or can we just return indexFiles.size() in the getter?

similarly, you can just write

this.totalSize = indexFiles.stream().mapToLong(idf -> idf.metadata().length()).sum();

ywelsch · 2018-04-20T07:19:24Z

server/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java

+            try {
+                return Files.size(f);
+            } catch (IOException e) {
+                throw new RuntimeException(e.getMessage(), e);


maybe just throw new UncheckedIOException(e). No need to pass the message as an extra parameter, it's just tests.

ywelsch · 2018-04-20T07:25:47Z

server/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java

+            .execute()
+            .actionGet();
+
+        final List<Path> files1 = scanSnapshotFolder(repoPath);


can you use a more descriptive variable name here? For example filesForSnapshot2

ywelsch · 2018-04-20T07:26:59Z

server/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java

+        }
+
+        // create another snapshot and drop 1st one
+        // total size has to grow, and has to be equal to files on fs


the first part of this comment is not true.

ywelsch · 2018-04-20T07:31:14Z

server/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java

+
+        SnapshotStats anotherStats = snapshots.get(0).getStats();
+
+        assertThat(anotherStats.getIncrementalFileCount(), equalTo(anotherStats.getProcessedFileCount()));


can you also verify that the incremental file count / size makes sense? This could be established by looking at which files have been added on the filesystem after creating the second snapshot.

This comment was not addressed, can you add more assertions here?

ywelsch · 2018-04-20T07:34:09Z

...r/src/main/java/org/elasticsearch/index/snapshots/blobstore/BlobStoreIndexShardSnapshot.java

+    }
+
+    /**
+     * Returns total number of files that where snapshotted


Maybe more exact:

Returns total number of files that are referenced by this snapshot

…k's PR

ywelsch

LGTM. Let's hold off on merging until we have figured out the BWC implications.

bleskes · 2018-05-10T07:40:41Z

When discussed this issue and decided to come up with BWC solution. Here is a suggestion of how we could do it while producing a semi-sane JSON in 6.x (bwc section can be removed in 7.0):

{
    "stats": {
        "incremental_files": {
        	"count": 8
        	"size": 4797
        	"size_in_bytes": "4.6kb",
        	"processed": 8,
        	"processed_size": "4.6kb",
        	"processed_size_in_bytes": "4797" 
	        "start_time_in_millis": 1523882347327,
    	    "time": "225ms",
	        "time_in_millis": 225
        },
        "all_files": {
        	"count": 11,
        	"size": "8kb",
        	"total_size_in_bytes": 4797
        }
        <--- bwc section --> 
        "total_file_count": 11,
        "processed_file_count": 8,
        "total_size": "8kb",
        "total_size_in_bytes": 8276,
        "processed_size": "4.6kb",
        "processed_size_in_bytes": 4797,
        "start_time_in_millis": 1523882347327,
        "time": "225ms",
        "time_in_millis": 225
    }
}

I also thought about a scheme where we distinguish between that files that have to be snapshotted vs files that have to be restored. I couldn't come up with good names with that scheme as snapshot means both a thing in the repository and the verb of copying files to the repository.

vladimirdolzhenko · 2018-05-11T10:23:25Z

@bleskes sounds good - but BWC section is not backward compatible with previous versions - I assume it would be better to keep it like this

"number_of_files": 8,
"processed_files": 8,
"total_size": "4.6kb",
"total_size_in_bytes": 4797,
"processed_size": "4.6kb",
"processed_size_in_bytes": 4797,
"start_time_in_millis": 1523882347327,
"time": "225ms",
"time_in_millis": 225

bleskes · 2018-05-11T10:37:22Z

Yeah, sorry. I copied the wrong section.

added nested objects "incremental_files" and "all_files" + keep bwc part

added consistent names for incremental snapshot + keep bwc part of BlobStoreIndexShardSnapshot

vladimirdolzhenko · 2018-05-11T11:08:20Z

@ywelsch i have couple questions to clarify with BlobStoreIndexShardSnapshot

do we need to change XContent semantic for it?
do we prefer manual XContent parsing or ConstructingObjectParser to handle that ?

ywelsch · 2018-05-11T11:43:09Z

@vladimirdolzhenko The internal storage format (BlobStoreIndexShardSnapshot) should not require any changes. I would just revert 7ab7262

vladimirdolzhenko · 2018-05-11T12:15:19Z

@bleskes we have another proposal - processed stats goes to its own nested object - it's optional and goes away if snapshot is fully processed; timings goes out of incremental (like we have before), and rename all_files to total :

{
    "stats": {
        "incremental": {
        	"count": 8
        	"size": 4797
        	"size_in_bytes": "4.6kb"
        },
       "processed": {
        	"count": 7,
        	"size": 4000,
        	"size_in_bytes": "4.0kb"
       },
        "total": {
        	"count": 11,
        	"size": "8kb",
        	"size_in_bytes": 4797
        },
        "start_time_in_millis": 1523882347327,
        "time": "225ms",
        "time_in_millis": 225,
        <--- bwc section --> 
       "number_of_files": 8,
       "processed_files": 8,
       "total_size": "4.6kb",
       "total_size_in_bytes": 4797,
       "processed_size": "4.6kb",
       "processed_size_in_bytes": 4797
    }
}

@ywelsch note: it is not possible to call nested object as processed_files as it is used in BWC section => therefore I suggest to use sections incremental, processed and total.

Any objections ?

ywelsch · 2018-05-23T12:58:08Z

docs/reference/migration/migrate_7_0/snapshotstats.asciidoc

+
+Snapshot stats details are provided in a new structured way:
+
+* `total` section for all the files that are referenced by the snapshot


dot at the end of sentence

ywelsch · 2018-05-23T12:58:32Z

docs/reference/migration/migrate_7_0/snapshotstats.asciidoc

+
+* `total` section for all the files that are referenced by the snapshot
+* `incremental` section for those files that actually needed to be copied over as part of the incremental snapshotting.
+* In case of a snapshot that's still in progress, there's also a`processed` section for files that are in the process of being copied.


space between a and processed

ywelsch · 2018-05-23T13:00:20Z

server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/status/SnapshotStats.java

-        builder.endObject();
-        return builder;
+        builder.startObject(Fields.STATS)
+            //  incremental starts


ywelsch

LGTM

vladimirdolzhenko · 2018-05-23T14:49:23Z

test this please

… is done)

vladimirdolzhenko · 2018-05-25T10:49:52Z

@ywelsch could you please have another quick look into BWC REST API part

vladimirdolzhenko · 2018-05-25T12:31:39Z

test this please

ywelsch · 2018-05-25T14:17:23Z

rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.status/10_basic.yml

+  - do:
+      snapshot.create:
+        repository: test_repo_status_1
+        snapshot: test_snapshot


I'm not sure if the tests will pass when you create a snapshot with name test_snapshot in this test and then another snapshot with same name in the other test.

reasonable point - but for some reason it passes locally - I think it would be better to use another name explicitly

I know why - ESClientYamlSuiteTestCase uses each test scenario at the same isolation way as scenarios in a different yml files -

elasticsearch/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java

Line 167 in a17d6ca

for (Path yamlFile : yamlFiles) {

Adds difference of number of files (and file sizes) between prev and current snapshot. Total number/size reflects total number/size of files in snapshot. Closes #18543 (cherry picked from commit 81eb8ba)

* master: silence InstallPluginCommandTests, see #30900 Remove left-over comment Fix double semicolon in import statement [TEST] Fix minor random bug from #30794 Include size of snapshot in snapshot metadata #18543, bwc clean up (#30890) Enabling testing against an external cluster (#30885) Add public key header/footer (#30877) SQL: Remove the last remaining server dependencies from jdbc (#30771) Include size of snapshot in snapshot metadata (#29602) Do not serialize basic license exp in x-pack info (#30848) Change BWC version for VerifyRepositoryResponse (#30796) [DOCS] Document index name limitations (#30826) Harmonize include_defaults tests (#30700)

* 6.x: Fix double semicolon in import statement [TEST] Fix minor random bug from #30794 Enabling testing against an external cluster (#30885) SQL: Remove the last remaining server dependencies from jdbc (#30771) Add public key header/footer (#30877) Include size of snapshot in snapshot metadata (#29602) QA: Test template creation during rolling restart (#30850) REST high-level client: add put ingest pipeline API (#30793) Do not serialize basic license exp in x-pack info (#30848) [docs] explainer for java packaging tests (#30825) Verify signatures on official plugins (#30800) [DOCS] Document index name limitations (#30826) [Docs] Add reindex.remote.whitelist example (#30828)

jpountz · 2018-05-30T09:02:22Z

@vladimirdolzhenko FYI rest-api-spec/src/main/resources/rest-api-spec/test/snapshot.status/10_basic.yml still mentions that this PR is pending a backport on the 6.x branch.

vladimirdolzhenko · 2018-05-30T09:35:43Z

thank you @jpountz, good catch, fixed

vladimirdolzhenko requested a review from ywelsch April 19, 2018 06:17

vladimirdolzhenko added the :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs label Apr 19, 2018

vladimirdolzhenko added v7.0.0 >breaking >enhancement and removed v7.0.0 labels Apr 19, 2018

Include size of snapshot in snapshot metadata

6bcfcb8

Adds difference of number of files (and file sizes) between prev and current snapshot. Total number/size reflects total number/size of files in snapshot. Closes elastic#18543

vladimirdolzhenko force-pushed the fix/18543 branch from b59c25b to 6bcfcb8 Compare April 19, 2018 08:44

ywelsch suggested changes Apr 19, 2018

View reviewed changes

Include size of snapshot in snapshot metadata - changes on Yannick's PR

a08c807

vladimirdolzhenko requested a review from ywelsch April 19, 2018 14:30

ywelsch suggested changes Apr 20, 2018

View reviewed changes

Include size of snapshot in snapshot metadata - changes on 2nd Yannic…

50d7c78

…k's PR

vladimirdolzhenko requested a review from ywelsch April 20, 2018 11:59

Include size of snapshot in snapshot metadata - changes on 3nd Yannic…

6ea7e01

…k's PR

ywelsch approved these changes Apr 20, 2018

View reviewed changes

vladimirdolzhenko changed the title ~~Include size of snapshot in snapshot metadata~~ Make snapshot metadata properties clear - total, processed and incremental (was total earlier) Apr 20, 2018

vladimirdolzhenko changed the title ~~Make snapshot metadata properties clear - total, processed and incremental (was total earlier)~~ Include size of snapshot in snapshot metadata Apr 24, 2018

vladimirdolzhenko self-assigned this May 10, 2018

Vladimir Dolzhenko added 2 commits May 11, 2018 12:46

Include size of snapshot in snapshot metadata elastic#29602

4ca0ba9

added nested objects "incremental_files" and "all_files" + keep bwc part

Include size of snapshot in snapshot metadata elastic#29602

7ab7262

added consistent names for incremental snapshot + keep bwc part of BlobStoreIndexShardSnapshot

vladimirdolzhenko requested a review from ywelsch May 11, 2018 10:55

Vladimir Dolzhenko added 2 commits May 23, 2018 09:45

Merge remote-tracking branch 'remotes/origin/master' into fix/18543

816c7d9

Merge remote-tracking branch 'remotes/origin/master' into fix/18543

d13d991

vladimirdolzhenko requested a review from ywelsch May 23, 2018 10:20

ywelsch reviewed May 23, 2018

View reviewed changes

ywelsch approved these changes May 23, 2018

View reviewed changes

typos fixed

76fe2ac

Vladimir Dolzhenko added 4 commits May 23, 2018 21:19

Merge remote-tracking branch 'remotes/origin/master' into fix/18543

c86c6e5

Merge remote-tracking branch 'remotes/origin/master' into fix/18543

4176564

added REST API test for snapshot/status (suspended before backporting…

c05b191

… is done)

Merge remote-tracking branch 'remotes/origin/master' into fix/18543

563bea7

added REST API test for snapshot/status with BWC fields

e1400a8

fixed matching of time_in_millis/start_time_in_millis

ccf7b22

vladimirdolzhenko requested a review from ywelsch May 25, 2018 14:00

ywelsch reviewed May 25, 2018

View reviewed changes

Vladimir Dolzhenko added 2 commits May 25, 2018 17:29

fix snapshot name for bwc test

f2a33d6

fix snapshot name for bwc test

7df83ef

vladimirdolzhenko merged commit 81eb8ba into elastic:master May 25, 2018

vladimirdolzhenko added the v6.4.0 label May 25, 2018

vladimirdolzhenko deleted the fix/18543 branch May 25, 2018 19:21

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019


		SnapshotStats anotherStats = snapshots.get(0).getStats();

		assertThat(anotherStats.getIncrementalFileCount(), equalTo(anotherStats.getProcessedFileCount()));


		Snapshot stats details are provided in a new structured way:

		* `total` section for all the files that are referenced by the snapshot

Include size of snapshot in snapshot metadata #29602

Include size of snapshot in snapshot metadata #29602

Conversation

vladimirdolzhenko commented Apr 19, 2018 • edited Loading

elasticmachine commented Apr 19, 2018

ywelsch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ywelsch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ywelsch left a comment

Choose a reason for hiding this comment

bleskes commented May 10, 2018

vladimirdolzhenko commented May 11, 2018

bleskes commented May 11, 2018

vladimirdolzhenko commented May 11, 2018

ywelsch commented May 11, 2018

vladimirdolzhenko commented May 11, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ywelsch left a comment

Choose a reason for hiding this comment

vladimirdolzhenko commented May 23, 2018

vladimirdolzhenko commented May 25, 2018

vladimirdolzhenko commented May 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jpountz commented May 30, 2018

vladimirdolzhenko commented May 30, 2018

vladimirdolzhenko commented Apr 19, 2018 •

edited

Loading

vladimirdolzhenko commented May 11, 2018 •

edited

Loading