Revise configuration related documents #905

brfrn169 · 2023-06-13T03:30:16Z

This PR revises the configuration related documents. Please take a look!

komamitsu · 2023-06-14T06:54:12Z

docs/getting-started-with-scalardb-on-cassandra.md

@@ -20,6 +20,9 @@ From here, we assume Oracle JDK 8 and Cassandra 3.11.x are properly installed in
 The **scalardb.properties** (getting-started/scalardb.properties) file holds the configuration for ScalarDB. Basically, it describes the Cassandra installation that will be used.

 ```properties
+# Cassandra storage implementation is used for Consensus Commit


Cassandra storage implementation

Cassandra storage adaptor or something might be better?

Good point. Currently, there is no consistent name for implementations of DistributedStorage. We call them storage implementations, storage adapters, or shims. I think we should unify this naming. Let's keep it as it is for now and unify it later in another PR. Thanks.

kota2and3kan

Thank you for the update!
I left some suggestions.
Please take a look when you have time!

docs/configurations.md

kota2and3kan · 2023-06-14T10:36:42Z

docs/configurations.md

+
+For the details of ScalarDB Server, see [ScalarDB Server](scalardb-server.md).
+
+### JDBC transactions


I think there are several notes for JDBC transactions as follows.

You can use JDBC transactions with the JDBC database only (i.e., we cannot use JDBC transactions with some NoSQL databases).

You cannot use JDBC transactions with Multi-storage transactions even if the all underlying databases are JDBC databases.

Especially, I think the second one (limitation for Multi-storage transactions) is a bit confusing. So, I think it would be better to add a note for Multi-storage transactions in this section.

What do you think?

Actually, as described in configurations.md, the storage property scalar.db.storage is specifically for the Consensus Commit transaction manager. So other transaction manager implementations (jdbc and grpc) do not use it.

Previously, this was not explicitly mentioned, but in the updated configurations.md included in this PR, I think we have made this clear. Given this, do you think we still need to add the special note for Multi-storage transactions?

Thank you for the explanation! I understood the scalar.db.storage is a dedicated configuration for Consensus Commit.

I didn't notice the scalar.db.storage is a specific configuration for Consensus Commit.

So, I think the special note is not necessary.

By the way, I was just curious, but if we specify both scalar.db.transaction_manager=jdbc and scalar.db.storage=mutli-storage, does ScalarDB ignore the values of scalar.db.storage?

Not for this PR, but I think it is probably better to throw an exception if scalar.db.transaction_manager=jdbc and scalar.db.storage are both set.

By the way, I was just curious, but if we specify both scalar.db.transaction_manager=jdbc and scalar.db.storage=mutli-storage, does ScalarDB ignore the values of scalar.db.storage?

@kota2and3kan Basically, that's correct. But, there is an exception for this. I will explain it along with my response to @feeblefakie's comment.

Not for this PR, but I think it is probably better to throw an exception if scalar.db.transaction_manager=jdbc and scalar.db.storage are both set.

@feeblefakie @kota2and3kan This is also related to @kota2and3kan's question.

In fact, there is a scenario where both configurations are used. This happens when you use both the JDBC transaction manager and the storage API directly. Although this seems like a very rare case, it's technically possible. While the JDBC transaction manager doesn't use the scalar.db.storage property, the property is used by the storage API.

@brfrn169
Ah, I see. Thank you for your answers!
I feel it is a little bit confusing for users...
So, it might be better to separate properties for DistributedTransaction and DistributedStorage explicitly in the future.
(But, basically, users don't use Storage API directly. So, it might be OK to keep the current behavior.)

docs/multi-storage-transactions.md

docs/two-phase-commit-transactions.md

Torch3333

LGTM, thank you!

brfrn169 · 2023-06-15T12:31:11Z

core/src/main/java/com/scalar/db/config/DatabaseConfig.java

@@ -88,7 +88,7 @@ protected void load() {
    metadataCacheExpirationTimeSecs =
        getLong(getProperties(), METADATA_CACHE_EXPIRATION_TIME_SECS, -1);
    activeTransactionManagementExpirationTimeMillis =
-        getLong(getProperties(), ACTIVE_TRANSACTION_MANAGEMENT_EXPIRATION_TIME_MILLIS, 0);
+        getLong(getProperties(), ACTIVE_TRANSACTION_MANAGEMENT_EXPIRATION_TIME_MILLIS, -1);


Changed the default value of the property scalar.db.active_transaction_management.expiration_time_millis for consistency. However, it's not backward incompatible change because values less than or equal to 0 are treated the same as before.

Co-authored-by: kota2and3kan <47254383+kota2and3kan@users.noreply.github.com>

kota2and3kan

LGTM! Thank you!

kota2and3kan · 2023-06-16T02:22:17Z

docs/configurations.md

+
+For the details of ScalarDB Server, see [ScalarDB Server](scalardb-server.md).
+
+### JDBC transactions


Thank you for the explanation! I understood the scalar.db.storage is a dedicated configuration for Consensus Commit.

I didn't notice the scalar.db.storage is a specific configuration for Consensus Commit.

So, I think the special note is not necessary.

By the way, I was just curious, but if we specify both scalar.db.transaction_manager=jdbc and scalar.db.storage=mutli-storage, does ScalarDB ignore the values of scalar.db.storage?

feeblefakie

Thank you!
I left some comments and suggestions. PTAL!

feeblefakie · 2023-06-16T02:28:19Z

docs/configurations.md

+
+For the details of ScalarDB Server, see [ScalarDB Server](scalardb-server.md).
+
+### JDBC transactions


Not for this PR, but I think it is probably better to throw an exception if scalar.db.transaction_manager=jdbc and scalar.db.storage are both set.

feeblefakie · 2023-06-16T02:31:45Z

docs/configurations.md

+
+For the details of the Multi-storage, see [Multi-storage Transactions](multi-storage-transactions.md).
+
+### ScalarDB Server (gRPC)


Shouldn't we mark it as deprecated? (or recommend users to use ScalarDB Cluster?)
Also, can you add a section for ScalarDB Cluster?

Good point. Actually, I was hesitant to mention ScalarDB Cluster here because this is the document of ScalarDB that is a community edition (OSS) product, but ScalarDB Cluster is an enterprise edition product. I think it would be good to discuss the differentiation between the community and enterprise editions. Thank you.

As per our offline discussion, I will proceed to mark ScalarDB Server as deprecated and will also add a note about ScalarDB Cluster in another PR. Thanks.

feeblefakie · 2023-06-16T02:32:00Z

docs/configurations.md

+In order to avoid frequent errors caused internally by the [`SQLITE_BUSY`](https://www.sqlite.org/rescode.html#busy),
+it is recommended to set a [`busy_timeout`](https://www.sqlite.org/c3ref/busy_timeout.html) parameter.
+
+## ScalarDB Server configurations


docs/configurations.md

Co-authored-by: Hiroyuki Yamada <mogwaing@gmail.com>

josh-wong

Looking good! I've left some suggestions and comments. PTAL!

docs/api-guide.md

josh-wong · 2023-06-16T05:38:25Z

docs/getting-started-with-scalardb-on-cosmosdb.md

 ```
 Note that you can use a primary key or a secondary key for `<COSMOS_DB_FOR_NOSQL_KEY>`.

+For the details of the configuration, please refer to [ScalarDB Configurations](configurations.md).


Suggested change

For the details of the configuration, please refer to [ScalarDB Configurations](configurations.md).

For details about configurations, see [ScalarDB Configurations](configurations.md).

josh-wong · 2023-06-16T05:39:44Z

docs/getting-started-with-scalardb-on-dynamodb.md

 ```

+For the details of the configuration, please refer to [ScalarDB Configurations](configurations.md).


Suggested change

For the details of the configuration, please refer to [ScalarDB Configurations](configurations.md).

For details about configurations, see [ScalarDB Configurations](configurations.md).

josh-wong · 2023-06-16T05:40:30Z

docs/getting-started-with-scalardb-on-jdbc.md

-```properties
-scalar.db.transaction_manager=jdbc
-```
+For the details of the configuration, please refer to [ScalarDB Configurations](configurations.md).


Suggested change

For the details of the configuration, please refer to [ScalarDB Configurations](configurations.md).

For details about configurations, see [ScalarDB Configurations](configurations.md).

josh-wong · 2023-06-16T05:42:40Z

docs/getting-started-with-scalardb-on-jdbc.md

Suggested change

Please follow [Getting Started with ScalarDB](getting-started-with-scalardb.md) to run the application.

To run the application, follow the instructions in [Getting Started with ScalarDB](getting-started-with-scalardb.md).

josh-wong · 2023-06-16T10:49:45Z

docs/configurations.md

+And an example of configurations for ScalarDB Server is as follows:
+```properties


Suggested change

And an example of configurations for ScalarDB Server is as follows:

```properties

And an example of configurations for ScalarDB Server is as follows:

```properties

josh-wong · 2023-06-16T10:50:40Z

docs/configurations.md

+scalar.db.contact_points=<ScalarDB Server host>
+
+# ScalarDB Server port
+scalar.db.contact_port=<ScalarDB Server port>


Suggested change

scalar.db.contact_port=<ScalarDB Server port>

scalar.db.contact_port=<SCALARDB_SERVER_PORT>

josh-wong · 2023-06-16T10:50:56Z

docs/configurations.md

+scalar.db.transaction_manager=grpc
+
+# ScalarDB Server host
+scalar.db.contact_points=<ScalarDB Server host>


Suggested change

scalar.db.contact_points=<ScalarDB Server host>

scalar.db.contact_points=<SCALARDB_SERVER_HOST>

josh-wong · 2023-06-16T10:51:22Z

docs/configurations.md

+In this case, an example of configurations for App is as follows:
+```properties


Suggested change

In this case, an example of configurations for App is as follows:

```properties

In this case, an example of configurations for the app is as follows:

```properties

josh-wong · 2023-06-16T10:53:36Z

docs/configurations.md

+In this setting, App (ScalarDB Library with gRPC) connects to an underlying storage/database through ScalarDB Server.
+This setting is recommended for production use.
+This is because ScalarDB Server implements [scalar-admin](https://github.com/scalar-labs/scalar-admin) interface, which enables you to take transactionally-consistent backups for ScalarDB by pausing the ScalarDB Server.


Suggested change

In this setting, App (ScalarDB Library with gRPC) connects to an underlying storage/database through ScalarDB Server.

This setting is recommended for production use.

This is because ScalarDB Server implements [scalar-admin](https://github.com/scalar-labs/scalar-admin) interface, which enables you to take transactionally-consistent backups for ScalarDB by pausing the ScalarDB Server.

In this configuration, the app (ScalarDB Library with gRPC) connects to an underlying storage/database through ScalarDB Server.

This configuration is recommended for production use because ScalarDB Server implements the [scalar-admin](https://github.com/scalar-labs/scalar-admin) interface, which enables you to take transactionally consistent backups for ScalarDB by pausing ScalarDB Server.

josh-wong

Looking good! I've left some suggestions and comments. PTAL!

Co-authored-by: Josh Wong <joshua.wong@scalar-labs.com>

brfrn169 · 2023-06-16T12:24:43Z

@josh-wong Thank you for the helpful suggestions! I have applied them and made some adjustments in fa39625. Please take a look again when you have time!

komamitsu · 2023-06-19T05:43:50Z

docs/configurations.md

+| Name                                       | Description                                                                                    | Default    |
+|--------------------------------------------|------------------------------------------------------------------------------------------------|------------|
+| `scalar.db.storage`                        | `cosmos` must be specified.                                                                    | -          |
+| `scalar.db.contact_points`                 | Azure Cosmos DB endpoint the SDK will connect to.                                              |            |


SDK sounds a bit weird to me since I'm wondering whether it means ScalarDB or not. How about ScalarDB instead? Sorry if I'm missing something.

Fixed in c09b36d. Thanks.

komamitsu · 2023-06-19T06:24:39Z

docs/configurations.md

+
+| Name                                                             | Description                                                                                                                                                                                                       | Default              |
+|------------------------------------------------------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------------------|
+| `scalar.db.metadata.cache_expiration_time_secs`                  | ScalarDB has a metadata cache to reduce the number of requests to the database. This setting specifies the expiration time of the cache in seconds.                                                               | `-1` (no expiration) |


This is not for this PR, but no expiration by default sounds a bit unsafe as, IIUC, other ScalarDB server instances keep old cache? 30 seconds by default or so might be safer

Indeed, that's a good point. I'll create another PR for this. Thanks.

komamitsu

LGTM! 👍

josh-wong

LGTM! Thank you🙌

feeblefakie

LGTM! Thank you!

brfrn169 · 2023-06-22T14:31:42Z

Pushed to the 3.9 branch as well.

@josh-wong I only pushed the changes from this PR to the 3.9 branch because I encountered a lot of conflicts when trying to merge them with other release branches.

josh-wong · 2023-06-23T09:03:15Z

@brfrn169 Thank you! I'll consider how we can address documentation in previous releases/branches.

Revise configuration related documents

b97d74f

brfrn169 requested review from komamitsu, feeblefakie, Torch3333, josh-wong and kota2and3kan June 13, 2023 03:30

brfrn169 self-assigned this Jun 13, 2023

brfrn169 added the documentation label Jun 13, 2023

komamitsu reviewed Jun 14, 2023

View reviewed changes

kota2and3kan reviewed Jun 14, 2023

View reviewed changes

Torch3333 approved these changes Jun 15, 2023

View reviewed changes

brfrn169 requested a review from komamitsu June 15, 2023 12:21

brfrn169 commented Jun 15, 2023

View reviewed changes

brfrn169 and others added 3 commits June 15, 2023 21:53

Apply suggestions from code review

13c4178

Co-authored-by: kota2and3kan <47254383+kota2and3kan@users.noreply.github.com>

Apply suggestions from code review

b5c8dc6

Co-authored-by: kota2and3kan <47254383+kota2and3kan@users.noreply.github.com>

Adjustments

c70fa78

brfrn169 requested a review from kota2and3kan June 15, 2023 13:31

kota2and3kan approved these changes Jun 16, 2023

View reviewed changes

feeblefakie reviewed Jun 16, 2023

View reviewed changes

Apply suggestions from code review

cbfa493

Co-authored-by: Hiroyuki Yamada <mogwaing@gmail.com>

brfrn169 requested a review from feeblefakie June 16, 2023 10:25

Adjustments

d38a758

brfrn169 force-pushed the revise-configuration-related-docs branch from 845b7a5 to d38a758 Compare June 16, 2023 10:28

josh-wong requested changes Jun 16, 2023

View reviewed changes

brfrn169 and others added 2 commits June 16, 2023 20:15

Apply suggestions from code review

48b4b6f

Co-authored-by: Josh Wong <joshua.wong@scalar-labs.com>

Adjustments

fa39625

brfrn169 requested a review from josh-wong June 16, 2023 12:22

komamitsu reviewed Jun 19, 2023

View reviewed changes

Fix based on feedback

c09b36d

brfrn169 requested a review from komamitsu June 19, 2023 07:11

komamitsu approved these changes Jun 19, 2023

View reviewed changes

josh-wong approved these changes Jun 19, 2023

View reviewed changes

feeblefakie approved these changes Jun 21, 2023

View reviewed changes

feeblefakie merged commit 527ce11 into master Jun 21, 2023
12 checks passed

feeblefakie deleted the revise-configuration-related-docs branch June 21, 2023 11:12

brfrn169 added a commit that referenced this pull request Jun 22, 2023

Revise configuration related documents (#905)

c4c44c4

brfrn169 added a commit that referenced this pull request Jul 7, 2023

Revise configuration related documents (#905)

b30e2db

This was referenced Sep 7, 2023

Remove unnecessary configurations doc for ScalarDB scalar-labs/docs-scalardb#72

Merged

Remove unnecessary configurations doc scalar-labs/docs-scalardb-community#51

Merged


		For the details of ScalarDB Server, see [ScalarDB Server](scalardb-server.md).

		### JDBC transactions


		For the details of the Multi-storage, see [Multi-storage Transactions](multi-storage-transactions.md).

		### ScalarDB Server (gRPC)

	For the details of the configuration, please refer to [ScalarDB Configurations](configurations.md).
	For details about configurations, see [ScalarDB Configurations](configurations.md).

	Please follow [Getting Started with ScalarDB](getting-started-with-scalardb.md) to run the application.
	To run the application, follow the instructions in [Getting Started with ScalarDB](getting-started-with-scalardb.md).

		And an example of configurations for ScalarDB Server is as follows:
		```properties

	scalar.db.contact_port=<ScalarDB Server port>
	scalar.db.contact_port=<SCALARDB_SERVER_PORT>

	scalar.db.contact_points=<ScalarDB Server host>
	scalar.db.contact_points=<SCALARDB_SERVER_HOST>

		In this case, an example of configurations for App is as follows:
		```properties

Revise configuration related documents #905

Revise configuration related documents #905

Conversation

brfrn169 commented Jun 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kota2and3kan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brfrn169 Jun 15, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brfrn169 Jun 16, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Torch3333 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kota2and3kan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feeblefakie 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

brfrn169 Jun 21, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josh-wong 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

josh-wong left a comment

Choose a reason for hiding this comment

brfrn169 commented Jun 16, 2023 • edited

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

komamitsu left a comment

Choose a reason for hiding this comment

josh-wong left a comment

Choose a reason for hiding this comment

feeblefakie left a comment

Choose a reason for hiding this comment

brfrn169 commented Jun 22, 2023

josh-wong commented Jun 23, 2023

brfrn169 Jun 15, 2023 •

edited

brfrn169 Jun 16, 2023 •

edited

brfrn169 Jun 21, 2023 •

edited

brfrn169 commented Jun 16, 2023 •

edited