Re: [DISCUSS] Allow UPDATE on settings virtual table to change running configuration
Hello everyone, It has been a long time since the last update on this thread, so I wanted to share some status updates: The issue is still awaiting review, but all my hopes are pinned on Benjamin :-) My question here is, is there anything I can do to facilitate the review for anyone who wants to delve into the patch? I have a few thoughts to follow: - CEPify the changes - this will allow us to see the result of the discussion on a single page without having to re-read the whole thread; - Write a blog post with possible design solutions - this will both reveal the results of the discussion and potentially will draw some attention to the community; - Presenting and discussing slides at one of the Cassandra Town Halls; I tend to the 1-st and/or 2-nd points. What are the best practices we have here for such cases though? Any thoughts? On Tue, 11 Jul 2023 at 15:51, Maxim Muzafarov wrote: > > Thank you for your comments and for sharing the ticket targeting > strategy, I'm really happy to see this page where I have found all the > answers to the questions I had. So, I tend towards your view and will > just land this ticket on the 5.0 release only for now as it makes > sense for me as well. > > I didn't add the feature flag for this feature because for 99% of the > source code changes it only works with Cassandra internals leaving the > public API unchanged. A few remarks on this are: > - the display format of the vtable property has changed to match the > yaml configuration style, this doesn't mean that we are displaying > property values in a completely different way in fact the formats > match with only 4 exceptions mentioned in the message above (this > should be fine for the major release I hope); > - a new column, which we've agreed to add (I'll fix the PR shortly); > > > I would also like to mention the follow-up todos required by this > issue to set the right expectations. Currently, we've brought a few > properties under the framework to make them updateable with the > SettingsTable, so that you can keep focusing on the framework itself > rather than on tagging the configuration properties themselves with > the @Mutable annotation. Although the solution is self-sufficient for > the already tagged properties, we still need to bring the rest of them > under the framework afterwards. I'll create an issue and do it right, > we'll be done with the inital patch. > > > On Fri, 7 Jul 2023 at 20:37, Josh McKenzie wrote: > > > > This really is great work Maxim; definitely appreciate all the hard work > > that's gone into it and I think the users will too. > > > > In terms of where it should land, we discussed this type of question at > > length on the ML awhile ago and ended up codifying it in the wiki: > > https://cwiki.apache.org/confluence/display/CASSANDRA/Patching%2C+versioning%2C+and+LTS+releases > > > > When working on a ticket, use the following guideline to determine which > > branch to apply it to (Note: See How To Commit for details on the commit > > and merge process) > > > > Bugfix: apply to oldest applicable LTS and merge up through latest GA to > > trunk > > > > In the event you need to make changes on the merge commit, merge with -s > > ours and revise the commit via --amend > > > > Improvement: apply to trunk only (next release) > > > > Note: refactoring and removing dead code qualifies as an Improvement; our > > priority is stability on GA lines > > > > New Feature: apply to trunk only (next release) > > > > Our priority is to keep the 2 LTS releases and latest GA stable while > > releasing new "latest GA" on a cadence that provides new improvements and > > functionality to users soon enough to be valuable and relevant. > > > > > > So in this case, target whatever unreleased next feature release (i.e. > > SEMVER MAJOR || MINOR) we have on deck. > > > > On Thu, Jul 6, 2023, at 1:21 PM, Ekaterina Dimitrova wrote: > > > > Hi, > > > > First of all, thank you for all the work! > > I personally think that it should be ok to add a new column. > > > > I will be very happy to see this landing in 5.0. > > I am personally against porting this patch to 4.1. To be clear, I am sure > > you did a great job and my response would be the same to every single > > person - the configuration is quite wide-spread and the devil is in the > > details. I do not see a good reason for exception here except convenience. > > There is no feature flag for these changes too, right? > > > > Best regards, > > Ekaterina > > > > На четвъртък, 6 юли 2023 г. Miklosovic, Stefan > > написа: > > > > Hi Maxim, > > > > I went through the PR and added my comments. I think David also reviewed > > it. All points you mentioned make sense to me but I humbly think it is > > necessary to have at least one additional pair of eyes on this as the patch > > is relatively impactful. > > > > I would like to see additional column in system_views.settings of name > > "mutable" and of type "boolean" to see what field I am actua
Re: [DISCUSS] CommitLog default disk access mode
+1 to adding the feature, clear and easy configurability, and if after a major cycle we can say with confidence it's beating the status quo in the vast majority of general cases, flip default. I mean, logically it *should* be, but infra software at the scale we do requires great care. :) This is great work Amit - well done. On Mon, Oct 16, 2023, at 4:28 PM, Dinesh Joshi wrote: > I haven't looked at the patch yet so take whatever I say here with a pinch of > salt. > > Philosophically, defaults should not change unless there is a clear > demonstrable benefit in majority cases for our users. In this case DirectIO > should have clear benefits. That said, this is a new feature and I would > personally default it to off. We should document it and allow for our users > to enable it. This derisks the project in case there is an inadvertent change > in behavior. > > Dinesh > >> On Oct 15, 2023, at 11:34 PM, Pawar, Amit wrote: >> >> [Public] >> >> >> Hi, >> >> CommitLog uses mmap (memory mapped ) segments by default. Direct-IO feature >> is proposed through new PR[1] to improve the CommitLog IO speed. Enabling >> this by default could be useful feature to address IO bottleneck seen during >> peak load. >> >> Need your input regarding changing this default. Please suggest. >> >> https://issues.apache.org/jira/browse/CASSANDRA-18464 >> >> thanks, >> Amit Pawar >> >> [1] - https://github.com/apache/cassandra/pull/2777
Re: [DISCUSS] CommitLog default disk access mode
I think introducing the feature is a good idea. I also think that it should _NOT_ be enabled by default for all the reasons stated above. Finding a cohort of users who are interested in turning it on would provide a nice testbed to shake out any issues without affecting everyone. On Tue, Oct 17, 2023 at 3:58 PM C. Scott Andreas wrote: > Let’s please not change the default at the same time the feature is > introduced. > > Making the capability available will allow users to evaluate and quantify > the benefit of the work, as well as to call out any unintended > consequences. As users and the project gain confidence in the results, we > can evaluate changing the default. > > – Scott > > On Oct 17, 2023, at 4:25 AM, guo Maxwell wrote: > > > -1 > > I still think we should keep it as it is until the direct io > for commitlog (read and write) is ready and relatively stable. And then we > may change the default value to direct io from mmap in a future version, > such as 5.2, or 6.0. > > Pawar, Amit 于2023年10月17日周二 19:03写道： > >> [AMD Official Use Only - General] >> >> Thank you all for your input. Received total 6 replies and below is the >> summary. >> >> >> >> 1. Mmap : 2/6 >> >> 2. Direct-I/O : 4/6 >> >> >> >> Default should be changed to Direct-IO then ? please confirm. >> >> >> >> Thanks, >> >> Amit >> >> >> >> Strongly agree with this point of view that direct IO can bring great >> benefits. >> >> >> >> I have reviewed part of the code, and my preliminary judgment is that it >> is not very common and limited in some situations, for example, it works >> for commitlog's write path only for this patch.So I suggest that the >> default value should not be modified until the entire function is >> comprehensive and stable, and then modified in a future version. >> >> >> >> Sam 于2023年10月17日周二 05:39写道： >> >> *Glad you brought up compaction here - I think there would be a >> significant benefit to moving compaction to direct i/o.* >> >> >> >> +1. Would love to see this get traction. >> >> >> >> On Mon, 16 Oct 2023 at 19:38, Jon Haddad >> wrote: >> >> Glad you brought up compaction here - I think there would be a >> significant benefit to moving compaction to direct i/o. >> >> >> On 2023/10/16 16:14:28 Benedict wrote: >> > I have some plans to (eventually) use the commit log as memtable >> payload storage (ie memtables would reference the commit log entries >> directly, storing only indexing info), and to back first level of sstables >> by reference to commit log entries. This will permit us to deliver not only >> much bigger memtables (cutting compaction throughput requirements by the >> ratio of size increase - so pretty dramatically), and faster flushing (so >> better behaviour ling write bursts), but also a fairly cheap and simple way >> to support MVCC - which will be helpful for transaction throughput. >> > >> > There is also a new commit log (“journal”) coming with Accord, that the >> rest of C* may or may not transition to. >> > >> > I only say this because this makes the utility of direct IO for commit >> log suspect, as we will be reading from the files as a matter of course >> should this go ahead; and we may end up relying on a different commit log >> implementation before long anyway. >> > >> > This is obviously a big suggestion and is not guaranteed to transpire, >> and probably won’t within the next year or so, but it should perhaps form >> some minimal part of any calculus. If the patch is otherwise simple and >> beneficial I don’t have anything against it, and the use of direct IO could >> well be of benefit eg in compaction - and also in future if we manage to >> bring a page management in process. So laying foundations there could be of >> benefit, even if the commit log eventually does not use it. >> > >> > > On 16 Oct 2023, at 17:00, Jon Haddad >> wrote: >> > > >> > > I haven't looked at the patch, but at a high level, defaulting to >> direct I/O for commit logs makes a lot of sense to me. >> > > >> > >> On 2023/10/16 06:34:05 "Pawar, Amit" wrote: >> > >> [Public] >> > >> >> > >> Hi, >> > >> >> > >> CommitLog uses mmap (memory mapped ) segments by default. Direct-IO >> feature is proposed through new PR[1] to improve the CommitLog IO speed. >> Enabling this by default could be useful feature to address IO bottleneck >> seen during peak load. >> > >> >> > >> Need your input regarding changing this default. Please suggest. >> > >> >> > >> https://issues.apache.org/jira/browse/CASSANDRA-18464 >> > >> >> > >> thanks, >> > >> Amit Pawar >> > >> >> > >> [1] - https://github.com/apache/cassandra/pull/2777 >> > >> >> > >> >> >> >> >> -- >> >> you are the apple of my eye ! >> > > > -- > you are the apple of my eye ! > >
Re: [DISCUSS] CEP-36: A Configurable ChannelProxy to alias external storage locations
If it is ok for Henrik to rebase the Astra implementation of this functionality (FileSystemProvider) onto Cassandra trunk. Then we can create a jira to move this forward for a small step. Claude Warren, Jr 于2023年10月18日周三 15:05写道： > Henrik and Guo, > > Have you moved forward on this topic? I have not seen anything recently. > I have posted a solution that intercepts calls for directories and injects > directories from different FileSystems. This means that a node can have > keyspaces both on the local file system and one or more other FileSystem > implementations. > > I look forward to hearing from you, > Claude > > > On Wed, Oct 18, 2023 at 9:00 AM Claude Warren, Jr > wrote: > >> After a bit more analysis and some testing I have a new branch that I >> think solves the problem. [1] I have also created a pull request internal >> to my clone so that it is easy to see the changes. [2] >> >> The strategy change is to move the insertion of the proxy from the >> Cassandra File class to the Directories class. This means that all action >> with the table is captured (this solves a problem encountered in the >> earlier strategy). >> The strategy is to create a path on a different FileSystem and return >> that. The example code only moves the data for the table to another >> directory on the same FileSystem but using a different FileSystem >> implementation should be a trivial change. >> >> The current code works on an entire keyspace. I, while code exists to >> limit the redirect to a table I have not tested that branch yet and am not >> certain that it will work. There is also some code (i.e. the PathParser) >> that may no longer be needed but has not been removed yet. >> >> Please take a look and let me know if you see any issues with this >> solution. >> >> Claude >> >> [1] https://github.com/Claudenw/cassandra/tree/FileSystemProxy >> [2] https://github.com/Claudenw/cassandra/pull/5/files >> >> >> >> On Tue, Oct 10, 2023 at 10:28 AM Claude Warren, Jr < >> claude.war...@aiven.io> wrote: >> >>> I have been exploring adding a second Path to the Cassandra File >>> object. The original path being the path within the standard Cassandra >>> directory tree and the second being a translated path when there is what >>> was called a ChannelProxy in place. >>> >>> A problem arises when the Directories.getLocationForDisk() is called. >>> It seems to be looking for locations that start with the data directory >>> absolute path. I can change it to make it look for the original path not >>> the translated path. But in other cases the translated path is the one >>> that is needed. >>> >>> I notice that there is a concept of multiple file locations in the code >>> base, particularly in the Directories.DataDirectories class where there are >>> "locationsForNonSystemKeyspaces" and "locationsForSystemKeyspace" in the >>> constructor, and in the >>> DatabaseDescriptor.getNonLocalSystemKeyspacesDataFileLocations() method >>> which returns an array of String and is populated from the cassandra.yaml >>> file. >>> >>> The DatabaseDescriptor.getNonLocalSystemKeyspacesDataFileLocations() >>> only ever seems to return an array of one item. >>> >>> Why does >>> DatabaseDescriptor.getNonLocalSystemKeyspacesDataFileLocations() return an >>> array? >>> >>> Should the system set the path to the root of the ColumnFamilyStore in >>> the ColumnFamilyStore directories instance? >>> Should the Directories.getLocationForDisk() do the proxy to the other >>> file system? >>> >>> Where is the proper location to change from the standard internal >>> representation to the remote location? >>> >>> >>> On Fri, Sep 29, 2023 at 8:07 AM Claude Warren, Jr < >>> claude.war...@aiven.io> wrote: >>> Sorry I was out sick and did not respond yesterday. Henrik, How does your system work? What is the design strategy? Also is your code available somewhere? After looking at the code some more I think that the best solution is not a FileChannelProxy but to modify the Cassandra File class to get a FileSystem object for a Factory to build the Path that is used within that object. I think that this makes if very small change that will pick up 90+% of the cases. We then just need to find the edge cases. On Fri, Sep 29, 2023 at 1:14 AM German Eichberger via dev < dev@cassandra.apache.org> wrote: > Super excited about this as well. Happy to help test with Azure and > any other way needed. > > Thanks, > German > -- > *From:* guo Maxwell > *Sent:* Wednesday, September 27, 2023 7:38 PM > *To:* dev@cassandra.apache.org > *Subject:* [EXTERNAL] Re: [DISCUSS] CEP-36: A Configurable > ChannelProxy to alias external storage locations > > Thanks , So I think a jira can be created now. And I'd be happy to > provide some help with this as well if needed. > > Henrik Ingo 于2023年9月28日周四 00:2
Re: ci-cassandra 5.0 and trunk now uses in-tree scripts
jvm-dtest-upgrade tests are currently broken in 5.0 and trunk. They are temporarily dummied out to keep the CI pipelines still working and not to otherwise block people. Please pay extra attention to them pre-commit until we figure out the breakage: CASSANDRA-18936 On Mon, 16 Oct 2023 at 17:58, Mick Semb Wever wrote: > FYI > > CASSANDRA-18665 has been committed. > This makes our ci-cassandra.a.o stage jobs on 5.0 and trunk branches build > and test with the in-tree script instead of cassandra-builds. > > This is a significant change, lots of testing happened, but I'm still > expecting some trouble (like everytime we touch ci-cassandra.a.o). Reach > out if there's anything. > > Next step is the standalone Jenkinsfile (which will also fix devbranch on > ci-cassandra.a.o on 5.0/trunk) and Jenkinsfile on k8s (repeatable CI, so > anyone can repeat ci-cassandra on their own k8s cluster). >
Re: [DISCUSS] CEP-36: A Configurable ChannelProxy to alias external storage locations
Henrik and Guo, Have you moved forward on this topic? I have not seen anything recently. I have posted a solution that intercepts calls for directories and injects directories from different FileSystems. This means that a node can have keyspaces both on the local file system and one or more other FileSystem implementations. I look forward to hearing from you, Claude On Wed, Oct 18, 2023 at 9:00 AM Claude Warren, Jr wrote: > After a bit more analysis and some testing I have a new branch that I > think solves the problem. [1] I have also created a pull request internal > to my clone so that it is easy to see the changes. [2] > > The strategy change is to move the insertion of the proxy from the > Cassandra File class to the Directories class. This means that all action > with the table is captured (this solves a problem encountered in the > earlier strategy). > The strategy is to create a path on a different FileSystem and return > that. The example code only moves the data for the table to another > directory on the same FileSystem but using a different FileSystem > implementation should be a trivial change. > > The current code works on an entire keyspace. I, while code exists to > limit the redirect to a table I have not tested that branch yet and am not > certain that it will work. There is also some code (i.e. the PathParser) > that may no longer be needed but has not been removed yet. > > Please take a look and let me know if you see any issues with this > solution. > > Claude > > [1] https://github.com/Claudenw/cassandra/tree/FileSystemProxy > [2] https://github.com/Claudenw/cassandra/pull/5/files > > > > On Tue, Oct 10, 2023 at 10:28 AM Claude Warren, Jr > wrote: > >> I have been exploring adding a second Path to the Cassandra File object. >> The original path being the path within the standard Cassandra directory >> tree and the second being a translated path when there is what was called a >> ChannelProxy in place. >> >> A problem arises when the Directories.getLocationForDisk() is called. It >> seems to be looking for locations that start with the data directory >> absolute path. I can change it to make it look for the original path not >> the translated path. But in other cases the translated path is the one >> that is needed. >> >> I notice that there is a concept of multiple file locations in the code >> base, particularly in the Directories.DataDirectories class where there are >> "locationsForNonSystemKeyspaces" and "locationsForSystemKeyspace" in the >> constructor, and in the >> DatabaseDescriptor.getNonLocalSystemKeyspacesDataFileLocations() method >> which returns an array of String and is populated from the cassandra.yaml >> file. >> >> The DatabaseDescriptor.getNonLocalSystemKeyspacesDataFileLocations() >> only ever seems to return an array of one item. >> >> Why does >> DatabaseDescriptor.getNonLocalSystemKeyspacesDataFileLocations() return an >> array? >> >> Should the system set the path to the root of the ColumnFamilyStore in >> the ColumnFamilyStore directories instance? >> Should the Directories.getLocationForDisk() do the proxy to the other >> file system? >> >> Where is the proper location to change from the standard internal >> representation to the remote location? >> >> >> On Fri, Sep 29, 2023 at 8:07 AM Claude Warren, Jr >> wrote: >> >>> Sorry I was out sick and did not respond yesterday. >>> >>> Henrik, How does your system work? What is the design strategy? Also >>> is your code available somewhere? >>> >>> After looking at the code some more I think that the best solution is >>> not a FileChannelProxy but to modify the Cassandra File class to get a >>> FileSystem object for a Factory to build the Path that is used within that >>> object. I think that this makes if very small change that will pick up >>> 90+% of the cases. We then just need to find the edge cases. >>> >>> >>> >>> >>> >>> On Fri, Sep 29, 2023 at 1:14 AM German Eichberger via dev < >>> dev@cassandra.apache.org> wrote: >>> Super excited about this as well. Happy to help test with Azure and any other way needed. Thanks, German -- *From:* guo Maxwell *Sent:* Wednesday, September 27, 2023 7:38 PM *To:* dev@cassandra.apache.org *Subject:* [EXTERNAL] Re: [DISCUSS] CEP-36: A Configurable ChannelProxy to alias external storage locations Thanks , So I think a jira can be created now. And I'd be happy to provide some help with this as well if needed. Henrik Ingo 于2023年9月28日周四 00:21写道： It seems I was volunteered to rebase the Astra implementation of this functionality (FileSystemProvider) onto Cassandra trunk. (And publish it, of course) I'll try to get going today or tomorrow, so that this discussion can then benefit from having that code available for inspection. And potentially using it as a soluttion to this use case. On Tue, Sep 26, 2023 at
Re: [DISCUSS] CEP-36: A Configurable ChannelProxy to alias external storage locations
After a bit more analysis and some testing I have a new branch that I think solves the problem. [1] I have also created a pull request internal to my clone so that it is easy to see the changes. [2] The strategy change is to move the insertion of the proxy from the Cassandra File class to the Directories class. This means that all action with the table is captured (this solves a problem encountered in the earlier strategy). The strategy is to create a path on a different FileSystem and return that. The example code only moves the data for the table to another directory on the same FileSystem but using a different FileSystem implementation should be a trivial change. The current code works on an entire keyspace. I, while code exists to limit the redirect to a table I have not tested that branch yet and am not certain that it will work. There is also some code (i.e. the PathParser) that may no longer be needed but has not been removed yet. Please take a look and let me know if you see any issues with this solution. Claude [1] https://github.com/Claudenw/cassandra/tree/FileSystemProxy [2] https://github.com/Claudenw/cassandra/pull/5/files On Tue, Oct 10, 2023 at 10:28 AM Claude Warren, Jr wrote: > I have been exploring adding a second Path to the Cassandra File object. > The original path being the path within the standard Cassandra directory > tree and the second being a translated path when there is what was called a > ChannelProxy in place. > > A problem arises when the Directories.getLocationForDisk() is called. It > seems to be looking for locations that start with the data directory > absolute path. I can change it to make it look for the original path not > the translated path. But in other cases the translated path is the one > that is needed. > > I notice that there is a concept of multiple file locations in the code > base, particularly in the Directories.DataDirectories class where there are > "locationsForNonSystemKeyspaces" and "locationsForSystemKeyspace" in the > constructor, and in the > DatabaseDescriptor.getNonLocalSystemKeyspacesDataFileLocations() method > which returns an array of String and is populated from the cassandra.yaml > file. > > The DatabaseDescriptor.getNonLocalSystemKeyspacesDataFileLocations() only > ever seems to return an array of one item. > > Why does DatabaseDescriptor.getNonLocalSystemKeyspacesDataFileLocations() > return an array? > > Should the system set the path to the root of the ColumnFamilyStore in the > ColumnFamilyStore directories instance? > Should the Directories.getLocationForDisk() do the proxy to the other file > system? > > Where is the proper location to change from the standard internal > representation to the remote location? > > > On Fri, Sep 29, 2023 at 8:07 AM Claude Warren, Jr > wrote: > >> Sorry I was out sick and did not respond yesterday. >> >> Henrik, How does your system work? What is the design strategy? Also >> is your code available somewhere? >> >> After looking at the code some more I think that the best solution is not >> a FileChannelProxy but to modify the Cassandra File class to get a >> FileSystem object for a Factory to build the Path that is used within that >> object. I think that this makes if very small change that will pick up >> 90+% of the cases. We then just need to find the edge cases. >> >> >> >> >> >> On Fri, Sep 29, 2023 at 1:14 AM German Eichberger via dev < >> dev@cassandra.apache.org> wrote: >> >>> Super excited about this as well. Happy to help test with Azure and any >>> other way needed. >>> >>> Thanks, >>> German >>> -- >>> *From:* guo Maxwell >>> *Sent:* Wednesday, September 27, 2023 7:38 PM >>> *To:* dev@cassandra.apache.org >>> *Subject:* [EXTERNAL] Re: [DISCUSS] CEP-36: A Configurable ChannelProxy >>> to alias external storage locations >>> >>> Thanks , So I think a jira can be created now. And I'd be happy to >>> provide some help with this as well if needed. >>> >>> Henrik Ingo 于2023年9月28日周四 00:21写道： >>> >>> It seems I was volunteered to rebase the Astra implementation of this >>> functionality (FileSystemProvider) onto Cassandra trunk. (And publish it, >>> of course) I'll try to get going today or tomorrow, so that this >>> discussion can then benefit from having that code available for inspection. >>> And potentially using it as a soluttion to this use case. >>> >>> On Tue, Sep 26, 2023 at 8:04 PM Jake Luciani wrote: >>> >>> We (DataStax) have a FileSystemProvider for Astra we can provide. >>> Works with S3/GCS/Azure. >>> >>> I'll ask someone on our end to make it accessible. >>> >>> This would work by having a bucket prefix per node. But there are lots >>> of details needed to support things like out of bound compaction >>> (mentioned in CEP). >>> >>> Jake >>> >>> On Tue, Sep 26, 2023 at 12:56 PM Benedict wrote: >>> > >>> > I agree with Ariel, the more suitable insertion point is probably the >>> JDK level FileSystemProvider and FileSystem abstr
